diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index fef32574f..bfcf5522d 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -2164,7 +2164,10 @@ mod tests { assert!(event_requires_delivery( &InProcessServerEvent::ServerNotification( codex_app_server_protocol::ServerNotification::ExternalAgentConfigImportCompleted( - codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification {}, + codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification { + import_id: "import".to_string(), + item_type_results: Vec::new(), + }, ) ) )); diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 1c37ea6f3..2c87ad5cd 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1139,8 +1139,122 @@ "type": "object" }, "ExternalAgentConfigImportCompletedNotification": { + "properties": { + "importId": { + "type": "string" + }, + "itemTypeResults": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportTypeResult" + }, + "type": "array" + } + }, + "required": [ + "importId", + "itemTypeResults" + ], "type": "object" }, + "ExternalAgentConfigImportItemTypeFailure": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "failureStage": { + "type": "string" + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "message": { + "type": "string" + }, + "source": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "failureStage", + "itemType", + "message" + ], + "type": "object" + }, + "ExternalAgentConfigImportItemTypeSuccess": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "source": { + "type": [ + "string", + "null" + ] + }, + "target": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "itemType" + ], + "type": "object" + }, + "ExternalAgentConfigImportTypeResult": { + "properties": { + "failures": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportItemTypeFailure" + }, + "type": "array" + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "successes": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportItemTypeSuccess" + }, + "type": "array" + } + }, + "required": [ + "failures", + "itemType", + "successes" + ], + "type": "object" + }, + "ExternalAgentConfigMigrationItemType": { + "enum": [ + "AGENTS_MD", + "CONFIG", + "SKILLS", + "PLUGINS", + "MCP_SERVER_CONFIG", + "SUBAGENTS", + "HOOKS", + "COMMANDS", + "SESSIONS" + ], + "type": "string" + }, "FileChangeOutputDeltaNotification": { "description": "Deprecated legacy notification for `apply_patch` textual output.\n\nThe server no longer emits this notification.", "properties": { 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 4aca0831c..81dbe61aa 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 @@ -8985,9 +8985,84 @@ }, "ExternalAgentConfigImportCompletedNotification": { "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "importId": { + "type": "string" + }, + "itemTypeResults": { + "items": { + "$ref": "#/definitions/v2/ExternalAgentConfigImportTypeResult" + }, + "type": "array" + } + }, + "required": [ + "importId", + "itemTypeResults" + ], "title": "ExternalAgentConfigImportCompletedNotification", "type": "object" }, + "ExternalAgentConfigImportItemTypeFailure": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "failureStage": { + "type": "string" + }, + "itemType": { + "$ref": "#/definitions/v2/ExternalAgentConfigMigrationItemType" + }, + "message": { + "type": "string" + }, + "source": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "failureStage", + "itemType", + "message" + ], + "type": "object" + }, + "ExternalAgentConfigImportItemTypeSuccess": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "itemType": { + "$ref": "#/definitions/v2/ExternalAgentConfigMigrationItemType" + }, + "source": { + "type": [ + "string", + "null" + ] + }, + "target": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "itemType" + ], + "type": "object" + }, "ExternalAgentConfigImportParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -9006,9 +9081,42 @@ }, "ExternalAgentConfigImportResponse": { "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "importId": { + "type": "string" + } + }, + "required": [ + "importId" + ], "title": "ExternalAgentConfigImportResponse", "type": "object" }, + "ExternalAgentConfigImportTypeResult": { + "properties": { + "failures": { + "items": { + "$ref": "#/definitions/v2/ExternalAgentConfigImportItemTypeFailure" + }, + "type": "array" + }, + "itemType": { + "$ref": "#/definitions/v2/ExternalAgentConfigMigrationItemType" + }, + "successes": { + "items": { + "$ref": "#/definitions/v2/ExternalAgentConfigImportItemTypeSuccess" + }, + "type": "array" + } + }, + "required": [ + "failures", + "itemType", + "successes" + ], + "type": "object" + }, "ExternalAgentConfigMigrationItem": { "properties": { "cwd": { 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 0249f2e07..ce874126f 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 @@ -5298,9 +5298,84 @@ }, "ExternalAgentConfigImportCompletedNotification": { "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "importId": { + "type": "string" + }, + "itemTypeResults": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportTypeResult" + }, + "type": "array" + } + }, + "required": [ + "importId", + "itemTypeResults" + ], "title": "ExternalAgentConfigImportCompletedNotification", "type": "object" }, + "ExternalAgentConfigImportItemTypeFailure": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "failureStage": { + "type": "string" + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "message": { + "type": "string" + }, + "source": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "failureStage", + "itemType", + "message" + ], + "type": "object" + }, + "ExternalAgentConfigImportItemTypeSuccess": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "source": { + "type": [ + "string", + "null" + ] + }, + "target": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "itemType" + ], + "type": "object" + }, "ExternalAgentConfigImportParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -5319,9 +5394,42 @@ }, "ExternalAgentConfigImportResponse": { "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "importId": { + "type": "string" + } + }, + "required": [ + "importId" + ], "title": "ExternalAgentConfigImportResponse", "type": "object" }, + "ExternalAgentConfigImportTypeResult": { + "properties": { + "failures": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportItemTypeFailure" + }, + "type": "array" + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "successes": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportItemTypeSuccess" + }, + "type": "array" + } + }, + "required": [ + "failures", + "itemType", + "successes" + ], + "type": "object" + }, "ExternalAgentConfigMigrationItem": { "properties": { "cwd": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json index b1a57704e..2b65371c6 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json @@ -1,5 +1,121 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "ExternalAgentConfigImportItemTypeFailure": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "failureStage": { + "type": "string" + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "message": { + "type": "string" + }, + "source": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "failureStage", + "itemType", + "message" + ], + "type": "object" + }, + "ExternalAgentConfigImportItemTypeSuccess": { + "properties": { + "cwd": { + "type": [ + "string", + "null" + ] + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "source": { + "type": [ + "string", + "null" + ] + }, + "target": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "itemType" + ], + "type": "object" + }, + "ExternalAgentConfigImportTypeResult": { + "properties": { + "failures": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportItemTypeFailure" + }, + "type": "array" + }, + "itemType": { + "$ref": "#/definitions/ExternalAgentConfigMigrationItemType" + }, + "successes": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportItemTypeSuccess" + }, + "type": "array" + } + }, + "required": [ + "failures", + "itemType", + "successes" + ], + "type": "object" + }, + "ExternalAgentConfigMigrationItemType": { + "enum": [ + "AGENTS_MD", + "CONFIG", + "SKILLS", + "PLUGINS", + "MCP_SERVER_CONFIG", + "SUBAGENTS", + "HOOKS", + "COMMANDS", + "SESSIONS" + ], + "type": "string" + } + }, + "properties": { + "importId": { + "type": "string" + }, + "itemTypeResults": { + "items": { + "$ref": "#/definitions/ExternalAgentConfigImportTypeResult" + }, + "type": "array" + } + }, + "required": [ + "importId", + "itemTypeResults" + ], "title": "ExternalAgentConfigImportCompletedNotification", "type": "object" } \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportResponse.json index 6823495d3..b1bed198a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportResponse.json @@ -1,5 +1,13 @@ { "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "importId": { + "type": "string" + } + }, + "required": [ + "importId" + ], "title": "ExternalAgentConfigImportResponse", "type": "object" } \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts index edb8f1916..4616157fc 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts @@ -1,5 +1,6 @@ // 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. +import type { ExternalAgentConfigImportTypeResult } from "./ExternalAgentConfigImportTypeResult"; -export type ExternalAgentConfigImportCompletedNotification = Record; +export type ExternalAgentConfigImportCompletedNotification = { importId: string, itemTypeResults: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportItemTypeFailure.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportItemTypeFailure.ts new file mode 100644 index 000000000..97386cba7 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportItemTypeFailure.ts @@ -0,0 +1,6 @@ +// 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. +import type { ExternalAgentConfigMigrationItemType } from "./ExternalAgentConfigMigrationItemType"; + +export type ExternalAgentConfigImportItemTypeFailure = { itemType: ExternalAgentConfigMigrationItemType, failureStage: string, message: string, cwd: string | null, source: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportItemTypeSuccess.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportItemTypeSuccess.ts new file mode 100644 index 000000000..d94ad4a9c --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportItemTypeSuccess.ts @@ -0,0 +1,6 @@ +// 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. +import type { ExternalAgentConfigMigrationItemType } from "./ExternalAgentConfigMigrationItemType"; + +export type ExternalAgentConfigImportItemTypeSuccess = { itemType: ExternalAgentConfigMigrationItemType, cwd: string | null, source: string | null, target: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportResponse.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportResponse.ts index 2ceddade0..19af89459 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportResponse.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportResponse.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type ExternalAgentConfigImportResponse = Record; +export type ExternalAgentConfigImportResponse = { importId: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportTypeResult.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportTypeResult.ts new file mode 100644 index 000000000..466e92f2d --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportTypeResult.ts @@ -0,0 +1,8 @@ +// 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. +import type { ExternalAgentConfigImportItemTypeFailure } from "./ExternalAgentConfigImportItemTypeFailure"; +import type { ExternalAgentConfigImportItemTypeSuccess } from "./ExternalAgentConfigImportItemTypeSuccess"; +import type { ExternalAgentConfigMigrationItemType } from "./ExternalAgentConfigMigrationItemType"; + +export type ExternalAgentConfigImportTypeResult = { itemType: ExternalAgentConfigMigrationItemType, successes: Array, failures: Array, }; 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 70efa3ab8..c0ad04658 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -107,8 +107,11 @@ export type { ExperimentalFeatureStage } from "./ExperimentalFeatureStage"; export type { ExternalAgentConfigDetectParams } from "./ExternalAgentConfigDetectParams"; export type { ExternalAgentConfigDetectResponse } from "./ExternalAgentConfigDetectResponse"; export type { ExternalAgentConfigImportCompletedNotification } from "./ExternalAgentConfigImportCompletedNotification"; +export type { ExternalAgentConfigImportItemTypeFailure } from "./ExternalAgentConfigImportItemTypeFailure"; +export type { ExternalAgentConfigImportItemTypeSuccess } from "./ExternalAgentConfigImportItemTypeSuccess"; export type { ExternalAgentConfigImportParams } from "./ExternalAgentConfigImportParams"; export type { ExternalAgentConfigImportResponse } from "./ExternalAgentConfigImportResponse"; +export type { ExternalAgentConfigImportTypeResult } from "./ExternalAgentConfigImportTypeResult"; export type { ExternalAgentConfigMigrationItem } from "./ExternalAgentConfigMigrationItem"; export type { ExternalAgentConfigMigrationItemType } from "./ExternalAgentConfigMigrationItemType"; export type { FeedbackUploadParams } from "./FeedbackUploadParams"; 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 d8d4a3af2..c4d113d1c 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -668,12 +668,47 @@ pub struct ExternalAgentConfigImportParams { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] -pub struct ExternalAgentConfigImportResponse {} +pub struct ExternalAgentConfigImportResponse { + pub import_id: String, +} #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] -pub struct ExternalAgentConfigImportCompletedNotification {} +pub struct ExternalAgentConfigImportItemTypeFailure { + pub item_type: ExternalAgentConfigMigrationItemType, + pub failure_stage: String, + pub message: String, + pub cwd: Option, + pub source: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ExternalAgentConfigImportItemTypeSuccess { + pub item_type: ExternalAgentConfigMigrationItemType, + pub cwd: Option, + pub source: Option, + pub target: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ExternalAgentConfigImportTypeResult { + pub item_type: ExternalAgentConfigMigrationItemType, + pub successes: Vec, + pub failures: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ExternalAgentConfigImportCompletedNotification { + pub import_id: String, + pub item_type_results: Vec, +} #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index c5f9e2f29..c342c832c 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -231,7 +231,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, 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 plugin/session migration items may additionally include structured `details` grouping plugin ids or session metadata. -- `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any plugin/session `details` returned by detect. When a request includes migration items, the server emits `externalAgentConfig/import/completed` once after the full import finishes (immediately after the response when everything completed synchronously, or after background imports finish). +- `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any plugin/session `details` returned by detect. Returns an `importId` used to correlate the completion notification. When a request includes migration items, the server emits `externalAgentConfig/import/completed` once after the full import finishes with type-level `itemResults` containing each migrated type's success count, error count, successes, and raw errors (immediately after the response when everything completed synchronously, or after background imports finish). - `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. - `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), the layered permission-profile allow map (`allowedPermissionProfiles`), the managed permission-profile default (`defaultPermissions`), lifecycle hook lockdown (`allowManagedHooksOnly`), remote-control policy (`allowRemoteControl`; `false` force-disables remote control while `true` or `null` preserves existing behavior), computer use policy (`computerUse`), pinned feature values (`featureRequirements`), managed lifecycle hooks (`hooks`), `enforceResidency`, and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`. diff --git a/codex-rs/app-server/src/config/external_agent_config.rs b/codex-rs/app-server/src/config/external_agent_config.rs index 665588213..7b7f0f2e9 100644 --- a/codex-rs/app-server/src/config/external_agent_config.rs +++ b/codex-rs/app-server/src/config/external_agent_config.rs @@ -82,6 +82,7 @@ pub(crate) struct MigrationDetails { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct PendingPluginImport { pub cwd: Option, + pub description: String, pub details: MigrationDetails, } @@ -91,6 +92,79 @@ pub(crate) struct PluginImportOutcome { pub succeeded_plugin_ids: Vec, pub failed_marketplaces: Vec, pub failed_plugin_ids: Vec, + pub raw_errors: Vec, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub(crate) struct ExternalAgentConfigImportOutcome { + pub pending_plugin_imports: Vec, + pub item_results: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ExternalAgentConfigImportItemResult { + pub item_type: ExternalAgentConfigMigrationItemType, + pub description: String, + pub cwd: Option, + pub success_count: u32, + pub error_count: u32, + pub successes: Vec, + pub raw_errors: Vec, +} + +impl ExternalAgentConfigImportItemResult { + pub(crate) fn new( + item_type: ExternalAgentConfigMigrationItemType, + description: String, + cwd: Option, + ) -> Self { + Self { + item_type, + description, + cwd, + success_count: 0, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + } + } + + pub(crate) fn record_successes(&mut self, count: usize) { + let count = u32::try_from(count).unwrap_or(u32::MAX); + self.success_count = self.success_count.saturating_add(count); + } + + pub(crate) fn record_error(&mut self, raw_error: ExternalAgentConfigImportRawError) { + self.error_count = self.error_count.saturating_add(1); + self.raw_errors.push(raw_error); + } + + pub(crate) fn record_success(&mut self, source: Option, target: Option) { + self.success_count = self.success_count.saturating_add(1); + self.successes.push(ExternalAgentConfigImportSuccess { + item_type: self.item_type, + cwd: self.cwd.clone(), + source, + target, + }); + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ExternalAgentConfigImportSuccess { + pub item_type: ExternalAgentConfigMigrationItemType, + pub cwd: Option, + pub source: Option, + pub target: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ExternalAgentConfigImportRawError { + pub item_type: ExternalAgentConfigMigrationItemType, + pub failure_stage: String, + pub message: String, + pub cwd: Option, + pub source: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -167,95 +241,175 @@ impl ExternalAgentConfigService { pub(crate) async fn import( &self, migration_items: Vec, - ) -> io::Result> { - let mut pending_plugin_imports = Vec::new(); + ) -> io::Result { + let mut outcome = ExternalAgentConfigImportOutcome::default(); for migration_item in migration_items { - match migration_item.item_type { - ExternalAgentConfigMigrationItemType::Config => { - self.import_config(migration_item.cwd.as_deref())?; + let item_type = migration_item.item_type; + let description = migration_item.description.clone(); + let cwd_for_log = migration_item.cwd.clone(); + let mut item_result = ExternalAgentConfigImportItemResult::new( + item_type, + description.clone(), + cwd_for_log.clone(), + ); + let import_result = match migration_item.item_type { + ExternalAgentConfigMigrationItemType::Config => (|| { + let migrated_count = self.import_config(migration_item.cwd.as_deref())?; emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::Config, /*skills_count*/ None, ); - } - ExternalAgentConfigMigrationItemType::Skills => { + item_result.record_successes(migrated_count); + Ok(()) + })(), + ExternalAgentConfigMigrationItemType::Skills => (|| { let skills_count = self.import_skills(migration_item.cwd.as_deref())?; emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::Skills, Some(skills_count), ); - } - ExternalAgentConfigMigrationItemType::AgentsMd => { - self.import_agents_md(migration_item.cwd.as_deref())?; + item_result.record_successes(skills_count); + Ok(()) + })(), + ExternalAgentConfigMigrationItemType::AgentsMd => (|| { + let migrated_count = self.import_agents_md(migration_item.cwd.as_deref())?; emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::AgentsMd, /*skills_count*/ None, ); - } + item_result.record_successes(migrated_count); + Ok(()) + })(), ExternalAgentConfigMigrationItemType::Plugins => { - let cwd = migration_item.cwd; - 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)?; + async { + let cwd = migration_item.cwd; + let details = match migration_item.details { + Some(details) => details, + None => { + let err = invalid_data_error( + "plugins migration item is missing details".to_string(), + ); + record_import_error( + &mut item_result, + "plugin_import", + err.to_string(), + /*source*/ None, + ); + return Err(err); + } + }; + let (local_details, remote_details) = match self + .partition_plugin_migration_details(cwd.as_deref(), details) + { + Ok(details) => details, + Err(err) => { + record_import_error( + &mut item_result, + "plugin_import", + err.to_string(), + /*source*/ None, + ); + return Err(err); + } + }; - if let Some(local_details) = local_details { - self.import_plugins(cwd.as_deref(), Some(local_details)) - .await?; + if let Some(local_details) = local_details { + let plugin_outcome = match self + .import_plugins(cwd.as_deref(), Some(local_details)) + .await + { + Ok(plugin_outcome) => plugin_outcome, + Err(err) => { + record_import_error( + &mut item_result, + "plugin_import", + err.to_string(), + /*source*/ None, + ); + return Err(err); + } + }; + for plugin_id in plugin_outcome.succeeded_plugin_ids { + item_result + .record_success(Some(plugin_id.clone()), Some(plugin_id)); + } + for raw_error in plugin_outcome.raw_errors { + item_result.record_error(raw_error); + } + } + if let Some(remote_details) = remote_details { + outcome.pending_plugin_imports.push(PendingPluginImport { + cwd, + description: description.clone(), + details: remote_details, + }); + } + emit_migration_metric( + EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, + ExternalAgentConfigMigrationItemType::Plugins, + /*skills_count*/ None, + ); + Ok(()) } - 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, - /*skills_count*/ None, - ); + .await } - ExternalAgentConfigMigrationItemType::McpServerConfig => { - self.import_mcp_server_config(migration_item.cwd.as_deref())?; + ExternalAgentConfigMigrationItemType::McpServerConfig => (|| { + let migrated_count = + self.import_mcp_server_config(migration_item.cwd.as_deref())?; emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::McpServerConfig, /*skills_count*/ None, ); - } - ExternalAgentConfigMigrationItemType::Subagents => { + item_result.record_successes(migrated_count); + Ok(()) + })(), + ExternalAgentConfigMigrationItemType::Subagents => (|| { let subagents_count = self.import_subagents(migration_item.cwd.as_deref())?; emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::Subagents, Some(subagents_count), ); - } - ExternalAgentConfigMigrationItemType::Hooks => { - self.import_hooks(migration_item.cwd.as_deref())?; + item_result.record_successes(subagents_count); + Ok(()) + })(), + ExternalAgentConfigMigrationItemType::Hooks => (|| { + let migrated_count = self.import_hooks(migration_item.cwd.as_deref())?; emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::Hooks, /*skills_count*/ None, ); - } - ExternalAgentConfigMigrationItemType::Commands => { + item_result.record_successes(migrated_count); + Ok(()) + })(), + ExternalAgentConfigMigrationItemType::Commands => (|| { let commands_count = self.import_commands(migration_item.cwd.as_deref())?; emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::Commands, Some(commands_count), ); + item_result.record_successes(commands_count); + Ok(()) + })(), + ExternalAgentConfigMigrationItemType::Sessions => Ok(()), + }; + if let Err(err) = import_result { + if item_type == ExternalAgentConfigMigrationItemType::Plugins { + outcome.item_results.push(item_result); + continue; } - ExternalAgentConfigMigrationItemType::Sessions => {} + return Err(err); } + outcome.item_results.push(item_result); } - Ok(pending_plugin_imports) + Ok(outcome) } async fn detect_migrations( @@ -718,6 +872,16 @@ impl ExternalAgentConfigService { .remove(&marketplace_name) }); let Some(import_source) = import_source else { + let message = format!( + "external agent plugin marketplace source was not found: {marketplace_name}" + ); + record_plugin_import_errors( + &mut outcome, + cwd, + &plugin_ids, + "plugin_import", + message, + ); outcome.failed_marketplaces.push(marketplace_name); outcome.failed_plugin_ids.extend(plugin_ids); continue; @@ -733,6 +897,16 @@ impl ExternalAgentConfigService { let Some(marketplace_path) = find_marketplace_manifest_path( add_marketplace_outcome.installed_root.as_path(), ) else { + let message = format!( + "plugin marketplace manifest was not found after install: {marketplace_name}" + ); + record_plugin_import_errors( + &mut outcome, + cwd, + &plugin_ids, + "plugin_import", + message, + ); outcome.failed_marketplaces.push(marketplace_name); outcome.failed_plugin_ids.extend(plugin_ids); continue; @@ -742,7 +916,14 @@ impl ExternalAgentConfigService { .push(marketplace_name.clone()); marketplace_path } - Err(_) => { + Err(err) => { + record_plugin_import_errors( + &mut outcome, + cwd, + &plugin_ids, + "plugin_import", + err.to_string(), + ); outcome.failed_marketplaces.push(marketplace_name); outcome.failed_plugin_ids.extend(plugin_ids); continue; @@ -759,9 +940,16 @@ impl ExternalAgentConfigService { Ok(_) => outcome .succeeded_plugin_ids .push(format!("{plugin_name}@{marketplace_name}")), - Err(_) => outcome - .failed_plugin_ids - .push(format!("{plugin_name}@{marketplace_name}")), + Err(err) => { + let plugin_id = format!("{plugin_name}@{marketplace_name}"); + outcome.failed_plugin_ids.push(plugin_id.clone()); + outcome.raw_errors.push(plugin_import_raw_error( + cwd, + "plugin_import", + err.to_string(), + Some(plugin_id), + )); + } } } } @@ -769,7 +957,7 @@ impl ExternalAgentConfigService { Ok(outcome) } - fn import_config(&self, cwd: Option<&Path>) -> io::Result<()> { + fn import_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() { ( @@ -777,7 +965,7 @@ impl ExternalAgentConfigService { repo_root.join(".codex").join("config.toml"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { - return Ok(()); + return Ok(0); } else { ( self.external_agent_home.join("settings.json"), @@ -785,11 +973,11 @@ impl ExternalAgentConfigService { ) }; let Some(settings) = effective_external_settings(&source_settings)? else { - return Ok(()); + return Ok(0); }; let migrated = build_config_from_external(&settings)?; if is_empty_toml_table(&migrated) { - return Ok(()); + return Ok(0); } let Some(target_parent) = target_config.parent() else { @@ -798,7 +986,7 @@ impl ExternalAgentConfigService { fs::create_dir_all(target_parent)?; if !target_config.exists() { write_toml_file(&target_config, &migrated)?; - return Ok(()); + return Ok(1); } let existing_raw = fs::read_to_string(&target_config)?; @@ -811,14 +999,14 @@ impl ExternalAgentConfigService { let changed = merge_missing_toml_values(&mut existing, &migrated)?; if !changed { - return Ok(()); + return Ok(0); } write_toml_file(&target_config, &existing)?; - Ok(()) + Ok(1) } - fn import_mcp_server_config(&self, cwd: Option<&Path>) -> io::Result<()> { + 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() { ( @@ -826,7 +1014,7 @@ impl ExternalAgentConfigService { repo_root.join(".codex").join("config.toml"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { - return Ok(()); + return Ok(0); } else { ( self.external_agent_home.join("settings.json"), @@ -843,7 +1031,7 @@ impl ExternalAgentConfigService { settings.as_ref(), )?; if is_empty_toml_table(&migrated) { - return Ok(()); + return Ok(0); } let Some(target_parent) = target_config.parent() else { @@ -851,8 +1039,9 @@ impl ExternalAgentConfigService { }; fs::create_dir_all(target_parent)?; if !target_config.exists() { + let migrated_count = migrated_mcp_server_names(&migrated).len(); write_toml_file(&target_config, &migrated)?; - return Ok(()); + return Ok(migrated_count); } let existing_raw = fs::read_to_string(&target_config)?; @@ -862,10 +1051,11 @@ impl ExternalAgentConfigService { toml::from_str::(&existing_raw) .map_err(|err| invalid_data_error(format!("invalid existing config.toml: {err}")))? }; - if !merge_missing_mcp_servers(&mut existing, &migrated)?.is_empty() { + let merged_server_count = merge_missing_mcp_servers(&mut existing, &migrated)?.len(); + if merged_server_count > 0 { write_toml_file(&target_config, &existing)?; } - Ok(()) + Ok(merged_server_count) } fn import_subagents(&self, cwd: Option<&Path>) -> io::Result { @@ -886,7 +1076,7 @@ impl ExternalAgentConfigService { import_subagents(&source_agents, &target_agents) } - fn import_hooks(&self, cwd: Option<&Path>) -> io::Result<()> { + 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)? { ( @@ -894,7 +1084,7 @@ impl ExternalAgentConfigService { repo_root.join(".codex").join("hooks.json"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { - return Ok(()); + return Ok(0); } else { ( self.external_agent_home.clone(), @@ -902,8 +1092,10 @@ impl ExternalAgentConfigService { ) }; - import_hooks(&source_external_agent_dir, &target_hooks)?; - Ok(()) + Ok(usize::from(import_hooks( + &source_external_agent_dir, + &target_hooks, + )?)) } fn import_commands(&self, cwd: Option<&Path>) -> io::Result { @@ -964,14 +1156,14 @@ impl ExternalAgentConfigService { Ok(copied_count) } - fn import_agents_md(&self, cwd: Option<&Path>) -> io::Result<()> { + 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(()); + return Ok(0); }; (source_agents_md, repo_root.join("AGENTS.md")) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { - return Ok(()); + return Ok(0); } else { ( self.external_agent_home.join(EXTERNAL_AGENT_CONFIG_MD), @@ -981,7 +1173,7 @@ impl ExternalAgentConfigService { if !is_non_empty_text_file(&source_agents_md)? || !is_missing_or_empty_text_file(&target_agents_md)? { - return Ok(()); + return Ok(0); } let Some(target_parent) = target_agents_md.parent() else { @@ -989,7 +1181,8 @@ impl ExternalAgentConfigService { }; fs::create_dir_all(target_parent)?; - rewrite_and_copy_text_file(&source_agents_md, &target_agents_md) + rewrite_and_copy_text_file(&source_agents_md, &target_agents_md)?; + Ok(1) } } @@ -1611,11 +1804,8 @@ fn invalid_data_error(message: impl Into) -> io::Error { io::Error::new(io::ErrorKind::InvalidData, message.into()) } -fn migration_metric_tags( - item_type: ExternalAgentConfigMigrationItemType, - skills_count: Option, -) -> Vec<(&'static str, String)> { - let migration_type = match item_type { +fn migration_item_type_label(item_type: ExternalAgentConfigMigrationItemType) -> &'static str { + match item_type { ExternalAgentConfigMigrationItemType::Config => "config", ExternalAgentConfigMigrationItemType::Skills => "skills", ExternalAgentConfigMigrationItemType::AgentsMd => "agents_md", @@ -1625,8 +1815,62 @@ fn migration_metric_tags( ExternalAgentConfigMigrationItemType::Hooks => "hooks", ExternalAgentConfigMigrationItemType::Commands => "commands", ExternalAgentConfigMigrationItemType::Sessions => "sessions", - }; - let mut tags = vec![("migration_type", migration_type.to_string())]; + } +} + +pub(crate) fn record_import_error( + result: &mut ExternalAgentConfigImportItemResult, + failure_stage: &'static str, + message: impl Into, + source: Option, +) { + result.record_error(ExternalAgentConfigImportRawError { + item_type: result.item_type, + failure_stage: failure_stage.to_string(), + message: message.into(), + cwd: result.cwd.clone(), + source, + }); +} + +fn record_plugin_import_errors( + outcome: &mut PluginImportOutcome, + cwd: Option<&Path>, + plugin_ids: &[String], + failure_stage: &'static str, + message: impl Into, +) { + let message = message.into(); + outcome + .raw_errors + .extend(plugin_ids.iter().map(|plugin_id| { + plugin_import_raw_error(cwd, failure_stage, message.clone(), Some(plugin_id.clone())) + })); +} + +fn plugin_import_raw_error( + cwd: Option<&Path>, + failure_stage: &'static str, + message: String, + source: Option, +) -> ExternalAgentConfigImportRawError { + ExternalAgentConfigImportRawError { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + failure_stage: failure_stage.to_string(), + message, + cwd: cwd.map(Path::to_path_buf), + source, + } +} + +fn migration_metric_tags( + item_type: ExternalAgentConfigMigrationItemType, + skills_count: Option, +) -> Vec<(&'static str, String)> { + let mut tags = vec![( + "migration_type", + migration_item_type_label(item_type).to_string(), + )]; if matches!( item_type, ExternalAgentConfigMigrationItemType::Skills diff --git a/codex-rs/app-server/src/config/external_agent_config_tests.rs b/codex-rs/app-server/src/config/external_agent_config_tests.rs index 8c120fb51..6675dc32e 100644 --- a/codex-rs/app-server/src/config/external_agent_config_tests.rs +++ b/codex-rs/app-server/src/config/external_agent_config_tests.rs @@ -35,6 +35,23 @@ fn github_plugin_details() -> MigrationDetails { } } +fn assert_single_plugin_raw_error( + raw_errors: &[ExternalAgentConfigImportRawError], + failure_stage: &str, + source: &str, +) { + assert_eq!(raw_errors.len(), 1); + let raw_error = &raw_errors[0]; + assert_eq!( + raw_error.item_type, + ExternalAgentConfigMigrationItemType::Plugins + ); + assert_eq!(raw_error.failure_stage, failure_stage); + assert_eq!(raw_error.cwd, None); + assert_eq!(raw_error.source.as_deref(), Some(source)); + assert!(!raw_error.message.is_empty()); +} + #[tokio::test] async fn detect_home_lists_config_skills_and_agents_md() { let (_root, external_agent_home, codex_home) = fixture_paths(); @@ -926,7 +943,7 @@ async fn import_home_skips_empty_config_migration() { ) .expect("write settings"); - service_for_paths(external_agent_home, codex_home.clone()) + let outcome = service_for_paths(external_agent_home, codex_home.clone()) .import(vec![ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::Config, description: String::new(), @@ -936,6 +953,18 @@ async fn import_home_skips_empty_config_migration() { .await .expect("import"); + assert_eq!( + outcome.item_results, + vec![ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::Config, + description: String::new(), + cwd: None, + success_count: 0, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + }] + ); assert!(!codex_home.join("config.toml").exists()); } @@ -1002,7 +1031,27 @@ async fn import_local_plugins_returns_completed_status() { .await .expect("import"); - assert_eq!(outcome, Vec::::new()); + assert_eq!( + outcome.pending_plugin_imports, + Vec::::new() + ); + assert_eq!( + outcome.item_results, + vec![ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: String::new(), + cwd: None, + success_count: 1, + error_count: 0, + successes: vec![ExternalAgentConfigImportSuccess { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + cwd: None, + source: Some("cloudflare@my-plugins".to_string()), + target: Some("cloudflare@my-plugins".to_string()), + }], + raw_errors: 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")); @@ -1044,9 +1093,10 @@ async fn import_git_plugins_returns_pending_async_status() { .expect("import"); assert_eq!( - outcome, + outcome.pending_plugin_imports, vec![PendingPluginImport { cwd: None, + description: String::new(), details: MigrationDetails { plugins: vec![PluginsMigration { marketplace_name: "acme-tools".to_string(), @@ -1056,6 +1106,18 @@ async fn import_git_plugins_returns_pending_async_status() { }, }] ); + assert_eq!( + outcome.item_results, + vec![ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: String::new(), + cwd: None, + success_count: 0, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + }] + ); assert!(!codex_home.join("config.toml").exists()); } @@ -1140,7 +1202,7 @@ async fn import_repo_agents_md_rewrites_terms_and_skips_non_empty_targets() { ) .expect("write target"); - service_for_paths( + let outcome = service_for_paths( root.path().join(EXTERNAL_AGENT_DIR), root.path().join(".codex"), ) @@ -1161,6 +1223,29 @@ async fn import_repo_agents_md_rewrites_terms_and_skips_non_empty_targets() { .await .expect("import"); + assert_eq!( + outcome.item_results, + vec![ + ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: String::new(), + cwd: Some(repo_root.clone()), + success_count: 1, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + }, + ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: String::new(), + cwd: Some(repo_with_existing_target.clone()), + success_count: 0, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + }, + ] + ); assert_eq!( fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"), "Codex\nCodex\nCodex\nSee AGENTS.md\n" @@ -1184,7 +1269,7 @@ async fn import_repo_agents_md_overwrites_empty_targets() { .expect("write source"); fs::write(repo_root.join("AGENTS.md"), " \n\t").expect("write empty target"); - service_for_paths( + let outcome = service_for_paths( root.path().join(EXTERNAL_AGENT_DIR), root.path().join(".codex"), ) @@ -1197,6 +1282,18 @@ async fn import_repo_agents_md_overwrites_empty_targets() { .await .expect("import"); + assert_eq!( + outcome.item_results, + vec![ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: String::new(), + cwd: Some(repo_root.clone()), + success_count: 1, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + }] + ); assert_eq!( fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"), "Codex guidance" @@ -1265,7 +1362,7 @@ async fn import_repo_hooks_preserves_disabled_codex_hooks_feature() { ) .expect("write config"); - service_for_paths( + let outcome = service_for_paths( root.path().join(EXTERNAL_AGENT_DIR), root.path().join(".codex"), ) @@ -1278,6 +1375,18 @@ async fn import_repo_hooks_preserves_disabled_codex_hooks_feature() { .await .expect("import"); + assert_eq!( + outcome.item_results, + vec![ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::Hooks, + description: String::new(), + cwd: Some(repo_root.clone()), + success_count: 1, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + }] + ); assert_eq!( fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"), "[features]\ncodex_hooks = false\n" @@ -1329,7 +1438,7 @@ async fn import_repo_mcp_uses_home_settings_toggles_when_repo_settings_missing() ) .expect("write external agent project config"); - service_for_paths(external_agent_home, root.path().join(".codex")) + let outcome = service_for_paths(external_agent_home, root.path().join(".codex")) .import(vec![ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, description: String::new(), @@ -1339,6 +1448,18 @@ async fn import_repo_mcp_uses_home_settings_toggles_when_repo_settings_missing() .await .expect("import"); + assert_eq!( + outcome.item_results, + vec![ExternalAgentConfigImportItemResult { + item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, + description: String::new(), + cwd: Some(repo_root.clone()), + success_count: 1, + error_count: 0, + successes: Vec::new(), + raw_errors: Vec::new(), + }] + ); let config: TomlValue = toml::from_str( &fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"), ) @@ -1496,6 +1617,40 @@ async fn import_repo_uses_non_empty_external_agent_agents_source() { ); } +#[tokio::test] +async fn import_continues_after_failed_migration_item() { + let root = TempDir::new().expect("create tempdir"); + let repo_root = root.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).expect("create git"); + fs::write(repo_root.join(EXTERNAL_AGENT_CONFIG_MD), "Claude guidance").expect("write source"); + + service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .import(vec![ + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: "invalid plugin migration".to_string(), + cwd: Some(repo_root.clone()), + details: None, + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: "valid agents migration".to_string(), + cwd: Some(repo_root.clone()), + details: None, + }, + ]) + .await + .expect("import continues"); + + assert_eq!( + fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"), + "Codex guidance" + ); +} + #[test] fn migration_metric_tags_for_skills_include_skills_count() { assert_eq!( @@ -2058,14 +2213,17 @@ async fn import_plugins_requires_source_marketplace_details() { .await .expect("import plugins"); + assert_eq!(outcome.succeeded_marketplaces, Vec::::new()); + assert_eq!(outcome.succeeded_plugin_ids, Vec::::new()); + assert_eq!(outcome.failed_marketplaces, vec!["other-tools".to_string()]); assert_eq!( - outcome, - PluginImportOutcome { - succeeded_marketplaces: Vec::new(), - succeeded_plugin_ids: Vec::new(), - failed_marketplaces: vec!["other-tools".to_string()], - failed_plugin_ids: vec!["formatter@other-tools".to_string()], - } + outcome.failed_plugin_ids, + vec!["formatter@other-tools".to_string()] + ); + assert_single_plugin_raw_error( + &outcome.raw_errors, + "plugin_import", + "formatter@other-tools", ); } @@ -2094,15 +2252,14 @@ async fn import_plugins_defers_marketplace_source_validation_to_add_marketplace( .await .expect("import plugins"); + assert_eq!(outcome.succeeded_marketplaces, Vec::::new()); + assert_eq!(outcome.succeeded_plugin_ids, Vec::::new()); + assert_eq!(outcome.failed_marketplaces, vec!["acme-tools".to_string()]); assert_eq!( - outcome, - PluginImportOutcome { - succeeded_marketplaces: Vec::new(), - succeeded_plugin_ids: Vec::new(), - failed_marketplaces: vec!["acme-tools".to_string()], - failed_plugin_ids: vec!["formatter@acme-tools".to_string()], - } + outcome.failed_plugin_ids, + vec!["formatter@acme-tools".to_string()] ); + assert_single_plugin_raw_error(&outcome.raw_errors, "plugin_import", "formatter@acme-tools"); } #[tokio::test] @@ -2173,6 +2330,7 @@ async fn import_plugins_supports_external_agent_plugin_marketplace_layout() { succeeded_plugin_ids: vec!["cloudflare@my-plugins".to_string()], failed_marketplaces: Vec::new(), failed_plugin_ids: Vec::new(), + raw_errors: Vec::new(), } ); let config = fs::read_to_string(codex_home.join("config.toml")).expect("read config"); @@ -2367,6 +2525,7 @@ async fn import_plugins_supports_relative_external_agent_plugin_marketplace_path succeeded_plugin_ids: vec!["cloudflare@my-plugins".to_string()], failed_marketplaces: Vec::new(), failed_plugin_ids: Vec::new(), + raw_errors: Vec::new(), } ); let config = fs::read_to_string(codex_home.join("config.toml")).expect("read config"); @@ -2407,13 +2566,19 @@ async fn import_plugins_infers_external_official_marketplace_when_missing_from_s .expect("import plugins"); assert_eq!( - outcome, - PluginImportOutcome { - succeeded_marketplaces: vec![EXTERNAL_OFFICIAL_MARKETPLACE_NAME.to_string()], - succeeded_plugin_ids: Vec::new(), - failed_marketplaces: Vec::new(), - failed_plugin_ids: vec![format!("sample@{EXTERNAL_OFFICIAL_MARKETPLACE_NAME}")], - } + outcome.succeeded_marketplaces, + vec![EXTERNAL_OFFICIAL_MARKETPLACE_NAME.to_string()] + ); + assert_eq!(outcome.succeeded_plugin_ids, Vec::::new()); + assert_eq!(outcome.failed_marketplaces, Vec::::new()); + assert_eq!( + outcome.failed_plugin_ids, + vec![format!("sample@{EXTERNAL_OFFICIAL_MARKETPLACE_NAME}")] + ); + assert_single_plugin_raw_error( + &outcome.raw_errors, + "plugin_import", + &format!("sample@{EXTERNAL_OFFICIAL_MARKETPLACE_NAME}"), ); } @@ -2571,6 +2736,7 @@ async fn import_plugins_supports_project_relative_external_agent_plugin_marketpl succeeded_plugin_ids: vec!["cloudflare@my-plugins".to_string()], failed_marketplaces: Vec::new(), failed_plugin_ids: Vec::new(), + raw_errors: Vec::new(), } ); let config = fs::read_to_string(codex_home.join("config.toml")).expect("read config"); diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 27191e721..938d539a9 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -898,7 +898,10 @@ mod tests { )); assert!(server_notification_requires_delivery( &ServerNotification::ExternalAgentConfigImportCompleted( - ExternalAgentConfigImportCompletedNotification {}, + ExternalAgentConfigImportCompletedNotification { + import_id: "import".to_string(), + item_type_results: Vec::new(), + }, ) )); } diff --git a/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs b/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs index 5d4c0594e..29faf9a9b 100644 --- a/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs +++ b/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs @@ -1,22 +1,29 @@ 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::error_code::invalid_params; use crate::outgoing_message::ConnectionRequestId; use crate::outgoing_message::OutgoingMessageSender; use codex_app_server_protocol::CommandMigration; use codex_app_server_protocol::ExternalAgentConfigDetectParams; use codex_app_server_protocol::ExternalAgentConfigDetectResponse; use codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification; +use codex_app_server_protocol::ExternalAgentConfigImportItemTypeFailure as ProtocolImportFailure; +use codex_app_server_protocol::ExternalAgentConfigImportItemTypeSuccess as ProtocolImportSuccess; use codex_app_server_protocol::ExternalAgentConfigImportParams; use codex_app_server_protocol::ExternalAgentConfigImportResponse; +use codex_app_server_protocol::ExternalAgentConfigImportTypeResult as ProtocolImportTypeResult; use codex_app_server_protocol::ExternalAgentConfigMigrationItem; use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; use codex_app_server_protocol::HookMigration; @@ -34,6 +41,7 @@ use std::path::PathBuf; use super::ConfigRequestProcessor; use super::external_agent_session_import::ExternalAgentSessionImporter; +use uuid::Uuid; #[derive(Clone)] pub(crate) struct ExternalAgentConfigRequestProcessor { @@ -169,6 +177,7 @@ impl ExternalAgentConfigRequestProcessor { request_id: ConnectionRequestId, params: ExternalAgentConfigImportParams, ) -> Result<(), JSONRPCErrorError> { + let import_id = Uuid::new_v4().to_string(); let needs_runtime_refresh = migration_items_need_runtime_refresh(¶ms.migration_items); let has_migration_items = !params.migration_items.is_empty(); let has_plugin_imports = params.migration_items.iter().any(|item| { @@ -177,26 +186,37 @@ impl ExternalAgentConfigRequestProcessor { ExternalAgentConfigMigrationItemType::Plugins ) }); - let pending_session_imports = self.validate_pending_session_imports(¶ms)?; - let pending_plugin_imports = self.import_external_agent_config(params).await?; + let (pending_session_imports, session_validation_result) = + self.validate_pending_session_imports(¶ms); + let import_outcome = self.import_external_agent_config(params).await?; if needs_runtime_refresh { self.config_processor.handle_config_mutation().await; } self.outgoing - .send_response(request_id, ExternalAgentConfigImportResponse {}) + .send_response( + request_id, + ExternalAgentConfigImportResponse { + import_id: import_id.clone(), + }, + ) .await; if !has_migration_items { return Ok(()); } - let has_background_imports = - !pending_plugin_imports.is_empty() || !pending_session_imports.is_empty(); + let mut completed_item_results = Vec::new(); + if let Some(session_validation_result) = session_validation_result { + completed_item_results.push(session_validation_result); + } + for item_result in import_outcome.item_results { + completed_item_results.push(item_result); + } + + let has_background_imports = !import_outcome.pending_plugin_imports.is_empty() + || !pending_session_imports.is_empty(); if !has_background_imports { - self.outgoing - .send_server_notification(ServerNotification::ExternalAgentConfigImportCompleted( - ExternalAgentConfigImportCompletedNotification {}, - )) + send_completed_import_notification(&self.outgoing, import_id, &completed_item_results) .await; return Ok(()); } @@ -205,34 +225,62 @@ impl ExternalAgentConfigRequestProcessor { let plugin_processor = self.clone(); let outgoing = Arc::clone(&self.outgoing); let thread_manager = Arc::clone(&self.thread_manager); + let session_import_result = (!pending_session_imports.is_empty()).then(|| { + CoreImportItemResult::new( + CoreMigrationItemType::Sessions, + "Import sessions".to_string(), + /*cwd*/ None, + ) + }); + let pending_plugin_imports = import_outcome.pending_plugin_imports; tokio::spawn(async move { - let session_imports = session_importer.import_sessions(pending_session_imports); + let session_imports = async move { + let session_import_result = session_import_result?; + let item_result = session_importer + .import_sessions(pending_session_imports, session_import_result) + .await; + Some(item_result) + }; let plugin_imports = async move { + let mut item_results = Vec::new(); for pending_plugin_import in pending_plugin_imports { + let mut item_result = CoreImportItemResult::new( + CoreMigrationItemType::Plugins, + pending_plugin_import.description.clone(), + pending_plugin_import.cwd.clone(), + ); match plugin_processor .complete_pending_plugin_import(pending_plugin_import) .await { - Ok(()) => {} + Ok(plugin_outcome) => { + apply_plugin_outcome_to_item_result(&mut item_result, plugin_outcome); + } Err(error) => { - tracing::warn!( - error = %error.message, - "external agent config plugin import failed" + record_import_error( + &mut item_result, + "plugin_import", + error.message.clone(), + /*source*/ None, ); } } + item_results.push(item_result); } + item_results }; - tokio::join!(session_imports, plugin_imports); + let (session_result, plugin_results) = tokio::join!(session_imports, plugin_imports); + let mut background_item_results = Vec::new(); + if let Some(session_result) = session_result { + background_item_results.push(session_result); + } + background_item_results.extend(plugin_results); + completed_item_results.extend(background_item_results); if has_plugin_imports { thread_manager.plugins_manager().clear_cache(); thread_manager.skills_manager().clear_cache(); } - outgoing - .send_server_notification(ServerNotification::ExternalAgentConfigImportCompleted( - ExternalAgentConfigImportCompletedNotification {}, - )) - .await; + send_completed_import_notification(&outgoing, import_id, &completed_item_results).await; }); Ok(()) @@ -241,7 +289,7 @@ impl ExternalAgentConfigRequestProcessor { fn validate_pending_session_imports( &self, params: &ExternalAgentConfigImportParams, - ) -> Result, JSONRPCErrorError> { + ) -> (Vec, Option) { let sessions = params .migration_items .iter() @@ -259,32 +307,66 @@ impl ExternalAgentConfigRequestProcessor { title: session.title, }) .collect::>(); + if sessions.is_empty() { + return (Vec::new(), None); + } + let mut item_result = CoreImportItemResult::new( + CoreMigrationItemType::Sessions, + "Validate session imports".to_string(), + /*cwd*/ None, + ); let mut selected_session_paths = HashSet::new(); let mut selected_sessions = Vec::new(); for session in sessions { - let Some(canonical_path) = self + let canonical_path = match self .migration_service .external_agent_session_source_path(&session.path) - .map_err(|err| internal_error(err.to_string()))? - else { - return Err(session_not_detected_error(&session.path)); + { + Ok(Some(canonical_path)) => canonical_path, + Ok(None) => { + record_import_error( + &mut item_result, + "session_missing", + format!( + "external agent session was not detected for import: {}", + session.path.display() + ), + Some(session.path.display().to_string()), + ); + continue; + } + Err(err) => { + record_import_error( + &mut item_result, + "session_source_path", + err.to_string(), + Some(session.path.display().to_string()), + ); + continue; + } }; if selected_session_paths.insert(canonical_path) { selected_sessions.push(session); } } - Ok(selected_sessions) + (selected_sessions, Some(item_result)) } async fn import_external_agent_config( &self, params: ExternalAgentConfigImportParams, - ) -> Result, JSONRPCErrorError> { + ) -> Result { self.migration_service .import( params .migration_items .into_iter() + .filter(|migration_item| { + !matches!( + migration_item.item_type, + ExternalAgentConfigMigrationItemType::Sessions + ) + }) .map(|migration_item| CoreMigrationItem { item_type: match migration_item.item_type { ExternalAgentConfigMigrationItemType::Config => { @@ -374,18 +456,130 @@ impl ExternalAgentConfigRequestProcessor { async fn complete_pending_plugin_import( &self, pending_plugin_import: PendingPluginImport, - ) -> Result<(), JSONRPCErrorError> { + ) -> Result { self.migration_service .import_plugins( pending_plugin_import.cwd.as_deref(), Some(pending_plugin_import.details), ) .await - .map(|_| ()) .map_err(|err| internal_error(err.to_string())) } } +async fn send_completed_import_notification( + outgoing: &OutgoingMessageSender, + import_id: String, + item_results: &[CoreImportItemResult], +) { + let notification = completed_notification(import_id, item_results); + outgoing + .send_server_notification(ServerNotification::ExternalAgentConfigImportCompleted( + notification, + )) + .await; +} + +fn completed_notification( + import_id: String, + item_results: &[CoreImportItemResult], +) -> ExternalAgentConfigImportCompletedNotification { + let mut protocol_type_results: Vec = Vec::new(); + for item_result in item_results { + let item_raw_errors = item_result + .raw_errors + .iter() + .map(protocol_import_raw_error) + .collect::>(); + let item_successes = item_result + .successes + .iter() + .map(protocol_import_success) + .collect::>(); + let item_type = protocol_migration_item_type(item_result.item_type); + if let Some(type_result) = protocol_type_results + .iter_mut() + .find(|type_result| type_result.item_type == item_type) + { + type_result.successes.extend(item_successes); + type_result.failures.extend(item_raw_errors); + } else { + protocol_type_results.push(ProtocolImportTypeResult { + item_type, + successes: item_successes, + failures: item_raw_errors, + }); + } + } + protocol_type_results.sort_by_key(|type_result| match type_result.item_type { + ExternalAgentConfigMigrationItemType::Config => 0, + ExternalAgentConfigMigrationItemType::Skills => 1, + ExternalAgentConfigMigrationItemType::AgentsMd => 2, + ExternalAgentConfigMigrationItemType::Plugins => 3, + ExternalAgentConfigMigrationItemType::McpServerConfig => 4, + ExternalAgentConfigMigrationItemType::Subagents => 5, + ExternalAgentConfigMigrationItemType::Hooks => 6, + ExternalAgentConfigMigrationItemType::Commands => 7, + ExternalAgentConfigMigrationItemType::Sessions => 8, + }); + + ExternalAgentConfigImportCompletedNotification { + import_id, + item_type_results: protocol_type_results, + } +} + +fn protocol_import_success( + success: &crate::config::external_agent_config::ExternalAgentConfigImportSuccess, +) -> ProtocolImportSuccess { + ProtocolImportSuccess { + item_type: protocol_migration_item_type(success.item_type), + cwd: success.cwd.clone(), + source: success.source.clone(), + target: success.target.clone(), + } +} + +fn protocol_import_raw_error(raw_error: &CoreImportRawError) -> ProtocolImportFailure { + ProtocolImportFailure { + item_type: protocol_migration_item_type(raw_error.item_type), + failure_stage: raw_error.failure_stage.clone(), + message: raw_error.message.clone(), + cwd: raw_error.cwd.clone(), + source: raw_error.source.clone(), + } +} + +fn protocol_migration_item_type( + item_type: CoreMigrationItemType, +) -> ExternalAgentConfigMigrationItemType { + match item_type { + CoreMigrationItemType::Config => ExternalAgentConfigMigrationItemType::Config, + CoreMigrationItemType::Skills => ExternalAgentConfigMigrationItemType::Skills, + CoreMigrationItemType::AgentsMd => ExternalAgentConfigMigrationItemType::AgentsMd, + CoreMigrationItemType::Plugins => ExternalAgentConfigMigrationItemType::Plugins, + CoreMigrationItemType::McpServerConfig => { + ExternalAgentConfigMigrationItemType::McpServerConfig + } + CoreMigrationItemType::Subagents => ExternalAgentConfigMigrationItemType::Subagents, + CoreMigrationItemType::Hooks => ExternalAgentConfigMigrationItemType::Hooks, + CoreMigrationItemType::Commands => ExternalAgentConfigMigrationItemType::Commands, + CoreMigrationItemType::Sessions => ExternalAgentConfigMigrationItemType::Sessions, + } +} + +fn apply_plugin_outcome_to_item_result( + item_result: &mut CoreImportItemResult, + plugin_outcome: PluginImportOutcome, +) { + for plugin_id in plugin_outcome.succeeded_plugin_ids { + item_result.record_success(Some(plugin_id.clone()), Some(plugin_id)); + } + for raw_error in plugin_outcome.raw_errors { + item_result.record_error(raw_error); + } +} + fn migration_items_need_runtime_refresh(items: &[ExternalAgentConfigMigrationItem]) -> bool { items.iter().any(|item| { matches!( @@ -400,13 +594,6 @@ fn migration_items_need_runtime_refresh(items: &[ExternalAgentConfigMigrationIte }) } -fn session_not_detected_error(path: &std::path::Path) -> JSONRPCErrorError { - invalid_params(format!( - "external agent session was not detected for import: {}", - path.display() - )) -} - #[cfg(test)] #[path = "external_agent_config_processor_tests.rs"] mod external_agent_config_processor_tests; diff --git a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs b/codex-rs/app-server/src/request_processors/external_agent_session_import.rs index 4cec9bca1..d4abb155e 100644 --- a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs +++ b/codex-rs/app-server/src/request_processors/external_agent_session_import.rs @@ -26,6 +26,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 crate::config_manager::ConfigManager; const SESSION_IMPORT_CONCURRENCY: usize = 5; @@ -58,12 +60,22 @@ impl ExternalAgentSessionImporter { } } - pub(super) async fn import_sessions(&self, sessions: Vec) { + pub(super) async fn import_sessions( + &self, + sessions: Vec, + mut item_result: ExternalAgentConfigImportItemResult, + ) -> ExternalAgentConfigImportItemResult { if sessions.is_empty() { - return; + return item_result; } let Ok(_permit) = self.permits.acquire().await else { - return; + record_import_error( + &mut item_result, + "session_permit", + "external agent session import permit could not be acquired", + /*source*/ None, + ); + return item_result; }; let import_results = futures::stream::iter(sessions) .map(|session| { @@ -76,23 +88,33 @@ impl ExternalAgentSessionImporter { let mut completed_imports = Vec::new(); while let Some(result) = import_results.next().await { match result { - Ok(Some(completed_import)) => completed_imports.push(completed_import), + Ok(Some(completed_import)) => { + item_result.record_success( + Some(completed_import.source_path.display().to_string()), + Some(completed_import.imported_thread_id.to_string()), + ); + completed_imports.push(completed_import); + } Ok(None) => {} Err(failure) => { - tracing::warn!( - error = %failure.message, - path = %failure.source_path.display(), - "external agent session import failed" + record_import_error( + &mut item_result, + failure.stage, + failure.message.clone(), + Some(failure.source_path.display().to_string()), ); } } } if let Err(err) = record_completed_session_imports(&self.codex_home, completed_imports) { - tracing::warn!( - error = %err, - "external agent session import ledger update failed" + record_import_error( + &mut item_result, + "session_ledger_update", + err.to_string(), + /*source*/ None, ); } + item_result } async fn import_requested_session( @@ -106,6 +128,7 @@ impl ExternalAgentSessionImporter { .map_err(|message| SessionImportFailure { source_path: source_path.clone(), message, + stage: "session_prepare", })? else { return Ok(None); @@ -116,6 +139,7 @@ impl ExternalAgentSessionImporter { .map_err(|message| SessionImportFailure { source_path: pending_import.source_path.clone(), message, + stage: "session_persist", })?; Ok(Some(CompletedExternalAgentSessionImport { source_path: pending_import.source_path, @@ -258,4 +282,5 @@ impl ExternalAgentSessionImporter { struct SessionImportFailure { source_path: PathBuf, message: String, + stage: &'static str, } 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 b75d703c4..8b165a11e 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 @@ -5,9 +5,10 @@ use app_test_support::TestAppServer; use app_test_support::create_mock_responses_server_repeating_assistant; use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml; -use codex_app_server::INVALID_PARAMS_ERROR_CODE; use codex_app_server_protocol::ExternalAgentConfigDetectResponse; +use codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification; use codex_app_server_protocol::ExternalAgentConfigImportResponse; +use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::PluginListParams; @@ -32,6 +33,11 @@ use tokio::time::timeout; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(60); +fn assert_import_response(response: ExternalAgentConfigImportResponse) -> String { + assert!(!response.import_id.is_empty()); + response.import_id +} + #[tokio::test] async fn external_agent_config_import_sends_completion_notification_for_sync_only_import() -> Result<()> { @@ -61,13 +67,58 @@ async fn external_agent_config_import_sends_completion_notification_for_sync_onl ) .await??; let response: ExternalAgentConfigImportResponse = to_response(response)?; - assert_eq!(response, ExternalAgentConfigImportResponse {}); + let import_id = assert_import_response(response); let notification = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), ) .await??; assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + assert_eq!(completed.import_id, import_id); + + Ok(()) +} + +#[tokio::test] +async fn external_agent_config_import_returns_error_for_failed_sync_import() -> 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#"{"env":{"FOO":"bar"}}"#, + )?; + std::fs::write(codex_home.path().join("config.toml"), "invalid = [")?; + let home_dir = codex_home.path().display().to_string(); + let mut mcp = + TestAppServer::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": "CONFIG", + "description": "Import config", + "cwd": null + }] + })), + ) + .await?; + + let error: JSONRPCError = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!(error.error.code, -32603); + assert!( + error.error.message.contains("invalid existing config.toml"), + "unexpected error: {error:?}" + ); Ok(()) } @@ -148,13 +199,16 @@ async fn external_agent_config_import_sends_completion_notification_for_local_pl .await??; let response: ExternalAgentConfigImportResponse = to_response(response)?; - assert_eq!(response, ExternalAgentConfigImportResponse {}); + let import_id = assert_import_response(response); let notification = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), ) .await??; assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + assert_eq!(completed.import_id, import_id); let request_id = mcp .send_plugin_list_request(PluginListParams { @@ -236,13 +290,16 @@ async fn external_agent_config_import_sends_completion_notification_after_pendin ) .await??; let response: ExternalAgentConfigImportResponse = to_response(response)?; - assert_eq!(response, ExternalAgentConfigImportResponse {}); + let import_id = assert_import_response(response); let notification = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), ) .await??; assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + assert_eq!(completed.import_id, import_id); Ok(()) } @@ -318,13 +375,40 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { ) .await??; let response: ExternalAgentConfigImportResponse = to_response(response)?; - assert_eq!(response, ExternalAgentConfigImportResponse {}); + let import_id = assert_import_response(response); let notification = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), ) .await??; assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + assert_eq!(completed.import_id, import_id); + assert_eq!(completed.item_type_results.len(), 1); + let session_result = &completed.item_type_results[0]; + assert_eq!( + session_result.item_type, + ExternalAgentConfigMigrationItemType::Sessions + ); + assert_eq!(session_result.failures, Vec::new()); + assert_eq!(session_result.successes.len(), 1); + let session_success = &session_result.successes[0]; + assert_eq!( + session_success.item_type, + ExternalAgentConfigMigrationItemType::Sessions + ); + assert_eq!(session_success.cwd, None); + let session_source = std::fs::canonicalize(&session_path)?.display().to_string(); + assert_eq!( + session_success.source.as_deref(), + Some(session_source.as_str()) + ); + let imported_thread_id = session_success + .target + .as_deref() + .expect("session success should include imported thread id") + .to_string(); let request_id = mcp .send_thread_list_request(ThreadListParams { @@ -352,6 +436,7 @@ async fn external_agent_config_import_creates_session_rollouts() -> Result<()> { .first() .expect("expected imported thread") .clone(); + assert_eq!(imported_thread_id, thread.id.to_string()); assert_eq!(thread.preview, "first request"); assert_eq!(thread.name.as_deref(), Some("source session title")); @@ -583,13 +668,16 @@ async fn external_agent_config_import_accepts_detected_session_payload_after_res ) .await??; let response: ExternalAgentConfigImportResponse = to_response(response)?; - assert_eq!(response, ExternalAgentConfigImportResponse {}); + let import_id = assert_import_response(response); let notification = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), ) .await??; assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + assert_eq!(completed.import_id, import_id); let request_id = mcp .send_thread_list_request(ThreadListParams { @@ -670,13 +758,17 @@ async fn external_agent_config_import_skips_already_imported_session_versions() mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) .await??; - let _: ExternalAgentConfigImportResponse = to_response(response)?; + let response: ExternalAgentConfigImportResponse = to_response(response)?; + let import_id = assert_import_response(response); let notification = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), ) .await??; assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + assert_eq!(completed.import_id, import_id); } let request_id = mcp @@ -770,7 +862,7 @@ async fn external_agent_config_import_returns_before_background_session_import_f ) .await??; let response: ExternalAgentConfigImportResponse = to_response(response)?; - assert_eq!(response, ExternalAgentConfigImportResponse {}); + let import_id = assert_import_response(response); assert!( timeout( @@ -794,7 +886,7 @@ async fn external_agent_config_import_returns_before_background_session_import_f ) .await??; let response: ExternalAgentConfigImportResponse = to_response(response)?; - assert_eq!(response, ExternalAgentConfigImportResponse {}); + let duplicate_import_id = assert_import_response(response); let writer = tokio::spawn(async move { let mut file = tokio::fs::OpenOptions::new() @@ -805,19 +897,22 @@ async fn external_agent_config_import_returns_before_background_session_import_f }); timeout(DEFAULT_TIMEOUT, writer).await???; - let notification = timeout( - DEFAULT_TIMEOUT, - mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), - ) - .await??; - assert_eq!(notification.method, "externalAgentConfig/import/completed"); - - let notification = timeout( - DEFAULT_TIMEOUT, - mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), - ) - .await??; - assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let mut completed_import_ids = Vec::new(); + for _ in 0..2 { + let notification = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), + ) + .await??; + assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + completed_import_ids.push(completed.import_id); + } + completed_import_ids.sort(); + let mut expected_import_ids = vec![import_id, duplicate_import_id]; + expected_import_ids.sort(); + assert_eq!(completed_import_ids, expected_import_ids); let request_id = mcp .send_thread_list_request(ThreadListParams { @@ -845,92 +940,6 @@ async fn external_agent_config_import_returns_before_background_session_import_f Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn external_agent_config_import_rejects_undetected_session_paths() -> Result<()> { - let server = create_mock_responses_server_repeating_assistant("unused").await; - let codex_home = TempDir::new()?; - create_config_toml(codex_home.path(), &server.uri())?; - let project_root = codex_home.path().join("repo"); - let recent_timestamp = chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Secs, true); - let session_dir = codex_home.path().join(".claude/projects/repo"); - let detected_session_path = session_dir.join("detected.jsonl"); - let undetected_session_path = codex_home.path().join("outside.jsonl"); - std::fs::create_dir_all(&project_root)?; - std::fs::create_dir_all(&session_dir)?; - for path in [&detected_session_path, &undetected_session_path] { - std::fs::write( - path, - format!( - r#"{{"type":"user","cwd":"{}","timestamp":"{}","message":{{"content":"first request"}}}}"#, - project_root.display(), - recent_timestamp - ), - )?; - } - - let home_dir = codex_home.path().display().to_string(); - let mut mcp = - TestAppServer::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": "SESSIONS", - "description": "Migrate recent sessions", - "cwd": null, - "details": { - "sessions": [{ - "path": undetected_session_path, - "cwd": project_root, - "title": "first request" - }] - } - }] - })), - ) - .await?; - let err: JSONRPCError = timeout( - DEFAULT_TIMEOUT, - mcp.read_stream_until_error_message(RequestId::Integer(request_id)), - ) - .await??; - assert_eq!(err.error.code, INVALID_PARAMS_ERROR_CODE); - assert!( - err.error - .message - .contains("external agent session was not detected for import") - ); - - let request_id = mcp - .send_thread_list_request(ThreadListParams { - cursor: None, - limit: None, - sort_key: None, - sort_direction: None, - model_providers: None, - source_kinds: None, - archived: None, - cwd: None, - use_state_db_only: false, - search_term: None, - parent_thread_id: None, - }) - .await?; - let response: JSONRPCResponse = timeout( - DEFAULT_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(request_id)), - ) - .await??; - let response: ThreadListResponse = to_response(response)?; - assert_eq!(response.data, Vec::new()); - - Ok(()) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn external_agent_config_import_compacts_huge_session_before_first_follow_up() -> Result<()> { let server = responses::start_mock_server().await; @@ -1022,13 +1031,17 @@ async fn external_agent_config_import_compacts_huge_session_before_first_follow_ mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) .await??; - let _: ExternalAgentConfigImportResponse = to_response(response)?; + let response: ExternalAgentConfigImportResponse = to_response(response)?; + let import_id = assert_import_response(response); let notification = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), ) .await??; assert_eq!(notification.method, "externalAgentConfig/import/completed"); + let completed: ExternalAgentConfigImportCompletedNotification = + serde_json::from_value(notification.params.expect("completed params"))?; + assert_eq!(completed.import_id, import_id); let request_id = mcp .send_thread_list_request(ThreadListParams { diff --git a/codex-rs/memories/write/src/extensions/ad_hoc.rs b/codex-rs/memories/write/src/extensions/ad_hoc.rs index 9e77ba3ba..eebbe957a 100644 --- a/codex-rs/memories/write/src/extensions/ad_hoc.rs +++ b/codex-rs/memories/write/src/extensions/ad_hoc.rs @@ -1,5 +1,6 @@ use crate::memory_extensions_root; use std::path::Path; +use tokio::io::AsyncWriteExt; pub(super) const INSTRUCTIONS: &str = include_str!("../../templates/extensions/ad_hoc/instructions.md"); @@ -16,7 +17,8 @@ pub(super) async fn seed_instructions(memory_root: &Path) -> std::io::Result<()> .await { Ok(mut file) => { - tokio::io::AsyncWriteExt::write_all(&mut file, INSTRUCTIONS.as_bytes()).await + file.write_all(INSTRUCTIONS.as_bytes()).await?; + file.flush().await } Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => Ok(()), Err(err) => Err(err),