From cb8b1bbcd64bc12338893f3591ab1c150105e9d4 Mon Sep 17 00:00:00 2001 From: alexsong-oai Date: Tue, 28 Apr 2026 17:45:24 -0700 Subject: [PATCH] Support detect and import MCP, Subagents, hooks, commands from external (#19949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why This PR expands the migration path so Codex can detect and import MCP server config, hooks, commands, and subagents configs in a Codex-native shape. ## What changed - Added a `codex-external-agent-migration` crate that owns conversion logic for external-agent MCP servers, hooks, commands, and subagents. - Extended the app-server external-agent config detection/import API with migration item types for MCP server config, hooks, commands, and subagents. ## Migration strategy The migration is intentionally conservative: Codex only imports external-agent config that can be represented safely in Codex today. Unsupported or ambiguous config is skipped instead of being partially translated into behavior that may not match the source system. - **MCP servers**: import supported stdio and HTTP MCP server definitions into `mcp_servers`. Disabled servers and servers filtered out by source `enabledMcpjsonServers` / `disabledMcpjsonServers` are skipped. Project-scoped MCP entries from `.claude.json` are included when they match the repo path. - **Hooks**: import only supported command hooks into `.codex/hooks.json`. Unsupported hook features such as conditional groups, async handlers, prompt/http hooks, or unknown fields are skipped. Referenced hook scripts are copied into `.codex/hooks/`, preserving any existing target scripts. - **Commands**: import supported external commands as Codex skills under `.agents/skills/source-command-*`. Commands that rely on source runtime expansion such as `$ARGUMENTS`, `$1`, `@file` references, shell interpolation, or colliding generated names are skipped. - **Subagents**: import valid subagent Markdown files into `.codex/agents/*.toml` when they have the minimum Codex agent fields. Source model names are not migrated, so imported agents keep the user’s Codex default model; compatible reasoning effort and sandbox mode are migrated when present. - **Skills and project guidance**: copy missing skill directories into `.agents/skills` and migrate `CLAUDE.md` guidance into `AGENTS.md`, rewriting source-agent terminology to Codex terminology where appropriate. - **Detection details**: detected migration items include lightweight details for UI preview, such as MCP server names, hook event names, generated command skill names, and subagent names. Import still recomputes from disk instead of trusting details as the source of truth. - Adds focused coverage for the new migration behavior and app-server import flow. ## Verification - `cargo test -p codex-external-agent-migration` - `cargo test -p codex-hooks` - `cargo test -p codex-app-server external_agent_config` - `just bazel-lock-check` --- codex-rs/Cargo.lock | 13 + codex-rs/Cargo.toml | 2 + .../schema/json/ClientRequest.json | 75 + .../codex_app_server_protocol.schemas.json | 75 + .../codex_app_server_protocol.v2.schemas.json | 75 + .../v2/ExternalAgentConfigDetectResponse.json | 75 + .../v2/ExternalAgentConfigImportParams.json | 75 + .../schema/typescript/v2/CommandMigration.ts | 5 + .../ExternalAgentConfigMigrationItemType.ts | 2 +- .../schema/typescript/v2/HookMigration.ts | 5 + .../typescript/v2/McpServerMigration.ts | 5 + .../schema/typescript/v2/MigrationDetails.ts | 6 +- .../schema/typescript/v2/SubagentMigration.ts | 5 + .../schema/typescript/v2/index.ts | 4 + .../app-server-protocol/src/protocol/v2.rs | 49 +- codex-rs/app-server/Cargo.toml | 1 + .../src/config/external_agent_config.rs | 362 +++- .../src/config/external_agent_config_tests.rs | 897 ++++++-- .../src/external_agent_config_api.rs | 69 + codex-rs/app-server/src/message_processor.rs | 59 +- codex-rs/external-agent-migration/BUILD.bazel | 6 + codex-rs/external-agent-migration/Cargo.toml | 23 + codex-rs/external-agent-migration/src/lib.rs | 1921 +++++++++++++++++ codex-rs/hooks/src/lib.rs | 22 + .../src/external_agent_config_migration.rs | 2 +- 25 files changed, 3662 insertions(+), 171 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/CommandMigration.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/HookMigration.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/McpServerMigration.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/SubagentMigration.ts create mode 100644 codex-rs/external-agent-migration/BUILD.bazel create mode 100644 codex-rs/external-agent-migration/Cargo.toml create mode 100644 codex-rs/external-agent-migration/src/lib.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 700af4c4b..f219096e3 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1853,6 +1853,7 @@ dependencies = [ "codex-core-plugins", "codex-device-key", "codex-exec-server", + "codex-external-agent-migration", "codex-external-agent-sessions", "codex-features", "codex-feedback", @@ -2683,6 +2684,18 @@ dependencies = [ "syn 2.0.114", ] +[[package]] +name = "codex-external-agent-migration" +version = "0.0.0" +dependencies = [ + "codex-hooks", + "pretty_assertions", + "serde_json", + "serde_yaml", + "tempfile", + "toml 0.9.11+spec-1.1.0", +] + [[package]] name = "codex-external-agent-sessions" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index a0434dd6f..a9d6ea6c2 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -40,6 +40,7 @@ members = [ "exec-server", "execpolicy", "execpolicy-legacy", + "external-agent-migration", "external-agent-sessions", "keyring-store", "file-search", @@ -147,6 +148,7 @@ codex-exec = { path = "exec" } codex-file-system = { path = "file-system" } codex-exec-server = { path = "exec-server" } codex-execpolicy = { path = "execpolicy" } +codex-external-agent-migration = { path = "external-agent-migration" } codex-external-agent-sessions = { path = "external-agent-sessions" } codex-experimental-api-macros = { path = "codex-experimental-api-macros" } codex-features = { path = "features" } diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index e011a930e..c94a9254a 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -354,6 +354,17 @@ ], "type": "object" }, + "CommandMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "ConfigBatchWriteParams": { "properties": { "edits": { @@ -850,6 +861,9 @@ "SKILLS", "PLUGINS", "MCP_SERVER_CONFIG", + "SUBAGENTS", + "HOOKS", + "COMMANDS", "SESSIONS" ], "type": "string" @@ -1390,6 +1404,17 @@ }, "type": "object" }, + "HookMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "ImageDetail": { "enum": [ "auto", @@ -1696,6 +1721,17 @@ ], "type": "object" }, + "McpServerMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "McpServerOauthLoginParams": { "properties": { "name": { @@ -1779,6 +1815,27 @@ }, "MigrationDetails": { "properties": { + "commands": { + "default": [], + "items": { + "$ref": "#/definitions/CommandMigration" + }, + "type": "array" + }, + "hooks": { + "default": [], + "items": { + "$ref": "#/definitions/HookMigration" + }, + "type": "array" + }, + "mcpServers": { + "default": [], + "items": { + "$ref": "#/definitions/McpServerMigration" + }, + "type": "array" + }, "plugins": { "default": [], "items": { @@ -1792,6 +1849,13 @@ "$ref": "#/definitions/SessionMigration" }, "type": "array" + }, + "subagents": { + "default": [], + "items": { + "$ref": "#/definitions/SubagentMigration" + }, + "type": "array" } }, "type": "object" @@ -3158,6 +3222,17 @@ ], "type": "string" }, + "SubagentMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "TextElement": { "properties": { "byteRange": { 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 4b6d42920..1726a603f 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 @@ -6786,6 +6786,17 @@ ], "type": "string" }, + "CommandMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "Config": { "additionalProperties": true, "properties": { @@ -8347,6 +8358,9 @@ "SKILLS", "PLUGINS", "MCP_SERVER_CONFIG", + "SUBAGENTS", + "HOOKS", + "COMMANDS", "SESSIONS" ], "type": "string" @@ -9556,6 +9570,17 @@ ], "type": "string" }, + "HookMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "HookOutputEntry": { "properties": { "kind": { @@ -10469,6 +10494,17 @@ "title": "McpResourceReadResponse", "type": "object" }, + "McpServerMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "McpServerOauthLoginCompletedNotification": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -10793,6 +10829,27 @@ }, "MigrationDetails": { "properties": { + "commands": { + "default": [], + "items": { + "$ref": "#/definitions/v2/CommandMigration" + }, + "type": "array" + }, + "hooks": { + "default": [], + "items": { + "$ref": "#/definitions/v2/HookMigration" + }, + "type": "array" + }, + "mcpServers": { + "default": [], + "items": { + "$ref": "#/definitions/v2/McpServerMigration" + }, + "type": "array" + }, "plugins": { "default": [], "items": { @@ -10806,6 +10863,13 @@ "$ref": "#/definitions/v2/SessionMigration" }, "type": "array" + }, + "subagents": { + "default": [], + "items": { + "$ref": "#/definitions/v2/SubagentMigration" + }, + "type": "array" } }, "type": "object" @@ -14136,6 +14200,17 @@ } ] }, + "SubagentMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "TerminalInteractionNotification": { "$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 06a228f76..0e6d461ac 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 @@ -3285,6 +3285,17 @@ ], "type": "string" }, + "CommandMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "Config": { "additionalProperties": true, "properties": { @@ -4846,6 +4857,9 @@ "SKILLS", "PLUGINS", "MCP_SERVER_CONFIG", + "SUBAGENTS", + "HOOKS", + "COMMANDS", "SESSIONS" ], "type": "string" @@ -6166,6 +6180,17 @@ ], "type": "string" }, + "HookMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "HookOutputEntry": { "properties": { "kind": { @@ -7123,6 +7148,17 @@ "title": "McpResourceReadResponse", "type": "object" }, + "McpServerMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "McpServerOauthLoginCompletedNotification": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -7447,6 +7483,27 @@ }, "MigrationDetails": { "properties": { + "commands": { + "default": [], + "items": { + "$ref": "#/definitions/CommandMigration" + }, + "type": "array" + }, + "hooks": { + "default": [], + "items": { + "$ref": "#/definitions/HookMigration" + }, + "type": "array" + }, + "mcpServers": { + "default": [], + "items": { + "$ref": "#/definitions/McpServerMigration" + }, + "type": "array" + }, "plugins": { "default": [], "items": { @@ -7460,6 +7517,13 @@ "$ref": "#/definitions/SessionMigration" }, "type": "array" + }, + "subagents": { + "default": [], + "items": { + "$ref": "#/definitions/SubagentMigration" + }, + "type": "array" } }, "type": "object" @@ -12022,6 +12086,17 @@ } ] }, + "SubagentMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "TerminalInteractionNotification": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectResponse.json index 67f5a43a9..b61b7064a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectResponse.json @@ -1,6 +1,17 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "CommandMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "ExternalAgentConfigMigrationItem": { "properties": { "cwd": { @@ -40,12 +51,58 @@ "SKILLS", "PLUGINS", "MCP_SERVER_CONFIG", + "SUBAGENTS", + "HOOKS", + "COMMANDS", "SESSIONS" ], "type": "string" }, + "HookMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, + "McpServerMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "MigrationDetails": { "properties": { + "commands": { + "default": [], + "items": { + "$ref": "#/definitions/CommandMigration" + }, + "type": "array" + }, + "hooks": { + "default": [], + "items": { + "$ref": "#/definitions/HookMigration" + }, + "type": "array" + }, + "mcpServers": { + "default": [], + "items": { + "$ref": "#/definitions/McpServerMigration" + }, + "type": "array" + }, "plugins": { "default": [], "items": { @@ -59,6 +116,13 @@ "$ref": "#/definitions/SessionMigration" }, "type": "array" + }, + "subagents": { + "default": [], + "items": { + "$ref": "#/definitions/SubagentMigration" + }, + "type": "array" } }, "type": "object" @@ -101,6 +165,17 @@ "path" ], "type": "object" + }, + "SubagentMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" } }, "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json index 2f6baf14e..b26e9d187 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json @@ -1,6 +1,17 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "CommandMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "ExternalAgentConfigMigrationItem": { "properties": { "cwd": { @@ -40,12 +51,58 @@ "SKILLS", "PLUGINS", "MCP_SERVER_CONFIG", + "SUBAGENTS", + "HOOKS", + "COMMANDS", "SESSIONS" ], "type": "string" }, + "HookMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, + "McpServerMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + }, "MigrationDetails": { "properties": { + "commands": { + "default": [], + "items": { + "$ref": "#/definitions/CommandMigration" + }, + "type": "array" + }, + "hooks": { + "default": [], + "items": { + "$ref": "#/definitions/HookMigration" + }, + "type": "array" + }, + "mcpServers": { + "default": [], + "items": { + "$ref": "#/definitions/McpServerMigration" + }, + "type": "array" + }, "plugins": { "default": [], "items": { @@ -59,6 +116,13 @@ "$ref": "#/definitions/SessionMigration" }, "type": "array" + }, + "subagents": { + "default": [], + "items": { + "$ref": "#/definitions/SubagentMigration" + }, + "type": "array" } }, "type": "object" @@ -101,6 +165,17 @@ "path" ], "type": "object" + }, + "SubagentMigration": { + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" } }, "properties": { diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandMigration.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandMigration.ts new file mode 100644 index 000000000..fdf28f318 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandMigration.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 CommandMigration = { name: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigMigrationItemType.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigMigrationItemType.ts index 2a272b280..d8576937f 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigMigrationItemType.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigMigrationItemType.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 ExternalAgentConfigMigrationItemType = "AGENTS_MD" | "CONFIG" | "SKILLS" | "PLUGINS" | "MCP_SERVER_CONFIG" | "SESSIONS"; +export type ExternalAgentConfigMigrationItemType = "AGENTS_MD" | "CONFIG" | "SKILLS" | "PLUGINS" | "MCP_SERVER_CONFIG" | "SUBAGENTS" | "HOOKS" | "COMMANDS" | "SESSIONS"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookMigration.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookMigration.ts new file mode 100644 index 000000000..92ec2d3da --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookMigration.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 HookMigration = { name: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/McpServerMigration.ts b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerMigration.ts new file mode 100644 index 000000000..03c125109 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/McpServerMigration.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 McpServerMigration = { name: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/MigrationDetails.ts b/codex-rs/app-server-protocol/schema/typescript/v2/MigrationDetails.ts index 243ffe86e..4fe87eabd 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/MigrationDetails.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/MigrationDetails.ts @@ -1,7 +1,11 @@ // 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 { CommandMigration } from "./CommandMigration"; +import type { HookMigration } from "./HookMigration"; +import type { McpServerMigration } from "./McpServerMigration"; import type { PluginsMigration } from "./PluginsMigration"; import type { SessionMigration } from "./SessionMigration"; +import type { SubagentMigration } from "./SubagentMigration"; -export type MigrationDetails = { plugins: Array, sessions: Array, }; +export type MigrationDetails = { plugins: Array, sessions: Array, mcpServers: Array, hooks: Array, subagents: Array, commands: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/SubagentMigration.ts b/codex-rs/app-server-protocol/schema/typescript/v2/SubagentMigration.ts new file mode 100644 index 000000000..aaf6cf0d9 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/SubagentMigration.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 SubagentMigration = { name: string, }; 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 5753272da..7aa224b8c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -58,6 +58,7 @@ export type { CommandExecutionRequestApprovalParams } from "./CommandExecutionRe export type { CommandExecutionRequestApprovalResponse } from "./CommandExecutionRequestApprovalResponse"; export type { CommandExecutionSource } from "./CommandExecutionSource"; export type { CommandExecutionStatus } from "./CommandExecutionStatus"; +export type { CommandMigration } from "./CommandMigration"; export type { Config } from "./Config"; export type { ConfigBatchWriteParams } from "./ConfigBatchWriteParams"; export type { ConfigEdit } from "./ConfigEdit"; @@ -154,6 +155,7 @@ export type { HookCompletedNotification } from "./HookCompletedNotification"; export type { HookEventName } from "./HookEventName"; export type { HookExecutionMode } from "./HookExecutionMode"; export type { HookHandlerType } from "./HookHandlerType"; +export type { HookMigration } from "./HookMigration"; export type { HookOutputEntry } from "./HookOutputEntry"; export type { HookOutputEntryKind } from "./HookOutputEntryKind"; export type { HookPromptFragment } from "./HookPromptFragment"; @@ -209,6 +211,7 @@ export type { McpResourceReadResponse } from "./McpResourceReadResponse"; export type { McpServerElicitationAction } from "./McpServerElicitationAction"; export type { McpServerElicitationRequestParams } from "./McpServerElicitationRequestParams"; export type { McpServerElicitationRequestResponse } from "./McpServerElicitationRequestResponse"; +export type { McpServerMigration } from "./McpServerMigration"; export type { McpServerOauthLoginCompletedNotification } from "./McpServerOauthLoginCompletedNotification"; export type { McpServerOauthLoginParams } from "./McpServerOauthLoginParams"; export type { McpServerOauthLoginResponse } from "./McpServerOauthLoginResponse"; @@ -313,6 +316,7 @@ export type { SkillsListExtraRootsForCwd } from "./SkillsListExtraRootsForCwd"; export type { SkillsListParams } from "./SkillsListParams"; export type { SkillsListResponse } from "./SkillsListResponse"; export type { SortDirection } from "./SortDirection"; +export type { SubagentMigration } from "./SubagentMigration"; export type { TerminalInteractionNotification } from "./TerminalInteractionNotification"; export type { TextElement } from "./TextElement"; export type { TextPosition } from "./TextPosition"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index c5a300943..40fcf1fc4 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1092,6 +1092,15 @@ pub enum ExternalAgentConfigMigrationItemType { #[serde(rename = "MCP_SERVER_CONFIG")] #[ts(rename = "MCP_SERVER_CONFIG")] McpServerConfig, + #[serde(rename = "SUBAGENTS")] + #[ts(rename = "SUBAGENTS")] + Subagents, + #[serde(rename = "HOOKS")] + #[ts(rename = "HOOKS")] + Hooks, + #[serde(rename = "COMMANDS")] + #[ts(rename = "COMMANDS")] + Commands, #[serde(rename = "SESSIONS")] #[ts(rename = "SESSIONS")] Sessions, @@ -1121,11 +1130,47 @@ pub struct SessionMigration { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] +pub struct McpServerMigration { + pub name: String, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct HookMigration { + pub name: String, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct SubagentMigration { + pub name: String, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct CommandMigration { + pub name: String, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] pub struct MigrationDetails { #[serde(default)] pub plugins: Vec, #[serde(default)] pub sessions: Vec, + #[serde(default)] + pub mcp_servers: Vec, + #[serde(default)] + pub hooks: Vec, + #[serde(default)] + pub subagents: Vec, + #[serde(default)] + pub commands: Vec, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] @@ -7876,7 +7921,7 @@ mod tests { marketplace_name: "team-marketplace".to_string(), plugin_names: vec!["asana".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), } ); @@ -7913,7 +7958,7 @@ mod tests { marketplace_name: "team-marketplace".to_string(), plugin_names: vec!["asana".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }], } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index dbfbfdc5a..f2beb6a12 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -38,6 +38,7 @@ codex-core = { workspace = true } codex-core-plugins = { workspace = true } codex-device-key = { workspace = true } codex-exec-server = { workspace = true } +codex-external-agent-migration = { workspace = true } codex-external-agent-sessions = { workspace = true } codex-features = { workspace = true } codex-git-utils = { workspace = true } 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 85582e7a2..bf0b3576a 100644 --- a/codex-rs/app-server/src/config/external_agent_config.rs +++ b/codex-rs/app-server/src/config/external_agent_config.rs @@ -9,6 +9,15 @@ use codex_core_plugins::marketplace::find_marketplace_manifest_path; use codex_core_plugins::marketplace_add::MarketplaceAddRequest; use codex_core_plugins::marketplace_add::add_marketplace; use codex_core_plugins::marketplace_add::is_local_marketplace_source; +use codex_external_agent_migration::build_mcp_config_from_external; +use codex_external_agent_migration::count_missing_commands; +use codex_external_agent_migration::count_missing_subagents; +use codex_external_agent_migration::hook_migration_event_names; +use codex_external_agent_migration::import_commands; +use codex_external_agent_migration::import_hooks; +use codex_external_agent_migration::import_subagents; +use codex_external_agent_migration::missing_command_names; +use codex_external_agent_migration::missing_subagent_names; use codex_external_agent_sessions::ExternalAgentSessionMigration; use codex_external_agent_sessions::detect_recent_sessions; use codex_protocol::protocol::Product; @@ -43,6 +52,9 @@ pub(crate) enum ExternalAgentConfigMigrationItemType { AgentsMd, Plugins, McpServerConfig, + Subagents, + Hooks, + Commands, Sessions, } @@ -53,9 +65,18 @@ pub(crate) struct PluginsMigration { } #[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct NamedMigration { + pub name: String, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] pub(crate) struct MigrationDetails { pub plugins: Vec, pub sessions: Vec, + pub mcp_servers: Vec, + pub hooks: Vec, + pub subagents: Vec, + pub commands: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -182,7 +203,38 @@ impl ExternalAgentConfigService { /*skills_count*/ None, ); } - ExternalAgentConfigMigrationItemType::McpServerConfig => {} + ExternalAgentConfigMigrationItemType::McpServerConfig => { + 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 => { + 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())?; + emit_migration_metric( + EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, + ExternalAgentConfigMigrationItemType::Hooks, + /*skills_count*/ None, + ); + } + 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), + ); + } ExternalAgentConfigMigrationItemType::Sessions => {} } } @@ -241,6 +293,81 @@ impl ExternalAgentConfigService { } } + let source_root = self.source_root(repo_root); + let mcp_settings = self.mcp_settings(repo_root, settings.clone())?; + let migrated_mcp = build_mcp_config_from_external( + source_root.as_path(), + Some(self.external_agent_home.as_path()), + mcp_settings.as_ref(), + )?; + let mcp_server_names = migrated_mcp_server_names(&migrated_mcp); + if !is_empty_toml_table(&migrated_mcp) { + let mut should_include = true; + if target_config.exists() { + let existing_raw = fs::read_to_string(&target_config)?; + let mut existing = if existing_raw.trim().is_empty() { + TomlValue::Table(Default::default()) + } else { + toml::from_str::(&existing_raw).map_err(|err| { + invalid_data_error(format!("invalid existing config.toml: {err}")) + })? + }; + should_include = merge_missing_toml_values(&mut existing, &migrated_mcp)?; + } + + if should_include { + items.push(ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, + description: format!( + "Migrate MCP servers from {} into {}", + source_root.display(), + target_config.display() + ), + cwd: cwd.clone(), + details: Some(MigrationDetails { + mcp_servers: named_migrations(mcp_server_names.clone()), + ..Default::default() + }), + }); + emit_migration_metric( + EXTERNAL_AGENT_CONFIG_DETECT_METRIC, + ExternalAgentConfigMigrationItemType::McpServerConfig, + /*skills_count*/ None, + ); + } + } + + let source_external_agent_dir = repo_root.map_or_else( + || self.external_agent_home.clone(), + |repo_root| repo_root.join(EXTERNAL_AGENT_DIR), + ); + let target_hooks = repo_root.map_or_else( + || self.codex_home.join("hooks.json"), + |repo_root| repo_root.join(".codex").join("hooks.json"), + ); + let hook_event_names = + hook_migration_event_names(source_external_agent_dir.as_path(), &target_hooks)?; + if !hook_event_names.is_empty() && is_missing_or_empty_text_file(&target_hooks)? { + items.push(ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Hooks, + description: format!( + "Migrate hooks from {} to {}", + source_external_agent_dir.display(), + target_hooks.display() + ), + cwd: cwd.clone(), + details: Some(MigrationDetails { + hooks: named_migrations(hook_event_names), + ..Default::default() + }), + }); + emit_migration_metric( + EXTERNAL_AGENT_CONFIG_DETECT_METRIC, + ExternalAgentConfigMigrationItemType::Hooks, + /*skills_count*/ None, + ); + } + let source_skills = repo_root.map_or_else( || self.external_agent_home.join("skills"), |repo_root| repo_root.join(EXTERNAL_AGENT_DIR).join("skills"), @@ -268,6 +395,62 @@ impl ExternalAgentConfigService { ); } + let source_commands = source_external_agent_dir.join("commands"); + let target_command_skills = repo_root.map_or_else( + || self.home_target_skills_dir(), + |repo_root| repo_root.join(".agents").join("skills"), + ); + let commands_count = count_missing_commands(&source_commands, &target_command_skills)?; + if commands_count > 0 { + let command_names = missing_command_names(&source_commands, &target_command_skills)?; + items.push(ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Commands, + description: format!( + "Migrate commands from {} to {}", + source_commands.display(), + target_command_skills.display() + ), + cwd: cwd.clone(), + details: Some(MigrationDetails { + commands: named_migrations(command_names), + ..Default::default() + }), + }); + emit_migration_metric( + EXTERNAL_AGENT_CONFIG_DETECT_METRIC, + ExternalAgentConfigMigrationItemType::Commands, + Some(commands_count), + ); + } + + let source_subagents = source_external_agent_dir.join("agents"); + let target_subagents = repo_root.map_or_else( + || self.codex_home.join("agents"), + |repo_root| repo_root.join(".codex").join("agents"), + ); + let subagents_count = count_missing_subagents(&source_subagents, &target_subagents)?; + if subagents_count > 0 { + let subagent_names = missing_subagent_names(&source_subagents, &target_subagents)?; + items.push(ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Subagents, + description: format!( + "Migrate subagents from {} to {}", + source_subagents.display(), + target_subagents.display() + ), + cwd: cwd.clone(), + details: Some(MigrationDetails { + subagents: named_migrations(subagent_names), + ..Default::default() + }), + }); + emit_migration_metric( + EXTERNAL_AGENT_CONFIG_DETECT_METRIC, + ExternalAgentConfigMigrationItemType::Subagents, + Some(subagents_count), + ); + } + let source_agents_md = if let Some(repo_root) = repo_root { find_repo_agents_md_source(repo_root)? } else { @@ -357,8 +540,8 @@ impl ExternalAgentConfigService { ), cwd: None, details: Some(MigrationDetails { - plugins: Vec::new(), sessions, + ..Default::default() }), }); emit_migration_metric( @@ -379,6 +562,41 @@ impl ExternalAgentConfigService { .unwrap_or_else(|| PathBuf::from(".agents").join("skills")) } + fn mcp_settings( + &self, + repo_root: Option<&Path>, + source_settings: Option, + ) -> io::Result> { + if repo_root.is_some() && source_settings.is_none() { + let home_settings = self.external_agent_home.join("settings.json"); + match read_external_settings(&home_settings) { + Ok(settings) => Ok(settings), + Err(err) => { + tracing::warn!( + path = %home_settings.display(), + error = %err, + "ignoring invalid external agent home settings during repo MCP migration" + ); + Ok(None) + } + } + } else { + Ok(source_settings) + } + } + + fn source_root(&self, repo_root: Option<&Path>) -> PathBuf { + repo_root.map_or_else( + || { + self.external_agent_home + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| PathBuf::from(".")) + }, + Path::to_path_buf, + ) + } + fn detect_plugin_migration( &self, source_settings: &Path, @@ -445,11 +663,11 @@ impl ExternalAgentConfigService { let local_details = (!local_plugins.is_empty()).then_some(MigrationDetails { plugins: local_plugins, - sessions: Vec::new(), + ..Default::default() }); let remote_details = (!remote_plugins.is_empty()).then_some(MigrationDetails { plugins: remote_plugins, - sessions: Vec::new(), + ..Default::default() }); Ok((local_details, remote_details)) @@ -535,7 +753,8 @@ impl ExternalAgentConfigService { } fn import_config(&self, cwd: Option<&Path>) -> io::Result<()> { - let (source_settings, target_config) = if let Some(repo_root) = find_repo_root(cwd)? { + let repo_root = find_repo_root(cwd)?; + let (source_settings, target_config) = if let Some(repo_root) = repo_root.as_ref() { ( repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), repo_root.join(".codex").join("config.toml"), @@ -586,6 +805,112 @@ impl ExternalAgentConfigService { Ok(()) } + fn import_mcp_server_config(&self, cwd: Option<&Path>) -> io::Result<()> { + let repo_root = find_repo_root(cwd)?; + let (source_settings, target_config) = if let Some(repo_root) = repo_root.as_ref() { + ( + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), + repo_root.join(".codex").join("config.toml"), + ) + } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { + return Ok(()); + } else { + ( + self.external_agent_home.join("settings.json"), + self.codex_home.join("config.toml"), + ) + }; + let settings = self.mcp_settings( + repo_root.as_deref(), + read_external_settings(&source_settings)?, + )?; + let migrated = build_mcp_config_from_external( + self.source_root(repo_root.as_deref()).as_path(), + Some(self.external_agent_home.as_path()), + settings.as_ref(), + )?; + if is_empty_toml_table(&migrated) { + return Ok(()); + } + + let Some(target_parent) = target_config.parent() else { + return Err(invalid_data_error("config target path has no parent")); + }; + fs::create_dir_all(target_parent)?; + if !target_config.exists() { + write_toml_file(&target_config, &migrated)?; + return Ok(()); + } + + let existing_raw = fs::read_to_string(&target_config)?; + let mut existing = if existing_raw.trim().is_empty() { + TomlValue::Table(Default::default()) + } else { + toml::from_str::(&existing_raw) + .map_err(|err| invalid_data_error(format!("invalid existing config.toml: {err}")))? + }; + if merge_missing_toml_values(&mut existing, &migrated)? { + write_toml_file(&target_config, &existing)?; + } + Ok(()) + } + + fn import_subagents(&self, cwd: Option<&Path>) -> io::Result { + let (source_agents, target_agents) = if let Some(repo_root) = find_repo_root(cwd)? { + ( + repo_root.join(EXTERNAL_AGENT_DIR).join("agents"), + repo_root.join(".codex").join("agents"), + ) + } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { + return Ok(0); + } else { + ( + self.external_agent_home.join("agents"), + self.codex_home.join("agents"), + ) + }; + + import_subagents(&source_agents, &target_agents) + } + + fn import_hooks(&self, cwd: Option<&Path>) -> io::Result<()> { + let (source_external_agent_dir, target_hooks) = + if let Some(repo_root) = find_repo_root(cwd)? { + ( + repo_root.join(EXTERNAL_AGENT_DIR), + repo_root.join(".codex").join("hooks.json"), + ) + } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { + return Ok(()); + } else { + ( + self.external_agent_home.clone(), + self.codex_home.join("hooks.json"), + ) + }; + + import_hooks(&source_external_agent_dir, &target_hooks)?; + Ok(()) + } + + fn import_commands(&self, cwd: Option<&Path>) -> io::Result { + let (source_commands, target_skills) = if let Some(repo_root) = find_repo_root(cwd)? { + ( + repo_root.join(EXTERNAL_AGENT_DIR).join("commands"), + repo_root.join(".agents").join("skills"), + ) + } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { + return Ok(0); + } else { + ( + self.external_agent_home.join("commands"), + self.home_target_skills_dir(), + ) + }; + + import_commands(&source_commands, &target_skills) + } + fn import_skills(&self, cwd: Option<&Path>) -> io::Result { let (source_skills, target_skills) = if let Some(repo_root) = find_repo_root(cwd)? { ( @@ -730,7 +1055,7 @@ fn extract_plugin_migration_details( Some(MigrationDetails { plugins, - sessions: Vec::new(), + ..Default::default() }) } @@ -1167,6 +1492,21 @@ fn write_toml_file(path: &Path, value: &TomlValue) -> io::Result<()> { fs::write(path, format!("{}\n", serialized.trim_end())) } +fn migrated_mcp_server_names(value: &TomlValue) -> Vec { + value + .get("mcp_servers") + .and_then(TomlValue::as_table) + .map(|servers| servers.keys().cloned().collect()) + .unwrap_or_default() +} + +fn named_migrations(names: Vec) -> Vec { + names + .into_iter() + .map(|name| NamedMigration { name }) + .collect() +} + fn is_empty_toml_table(value: &TomlValue) -> bool { match value { TomlValue::Table(table) => table.is_empty(), @@ -1193,10 +1533,18 @@ fn migration_metric_tags( ExternalAgentConfigMigrationItemType::AgentsMd => "agents_md", ExternalAgentConfigMigrationItemType::Plugins => "plugins", ExternalAgentConfigMigrationItemType::McpServerConfig => "mcp_server_config", + ExternalAgentConfigMigrationItemType::Subagents => "subagents", + ExternalAgentConfigMigrationItemType::Hooks => "hooks", + ExternalAgentConfigMigrationItemType::Commands => "commands", ExternalAgentConfigMigrationItemType::Sessions => "sessions", }; let mut tags = vec![("migration_type", migration_type.to_string())]; - if item_type == ExternalAgentConfigMigrationItemType::Skills { + if matches!( + item_type, + ExternalAgentConfigMigrationItemType::Skills + | ExternalAgentConfigMigrationItemType::Subagents + | ExternalAgentConfigMigrationItemType::Commands + ) { tags.push(("skills_count", skills_count.unwrap_or(0).to_string())); } tags 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 da0b7a2a9..208226135 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 @@ -3,9 +3,17 @@ use pretty_assertions::assert_eq; use std::io; use tempfile::TempDir; +const EXTERNAL_AGENT_PROJECT_CONFIG_FILE: &str = ".claude.json"; +const EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR: &str = ".claude-plugin"; +const SOURCE_EXTERNAL_AGENT_NAME: &str = "claude"; +const SOURCE_EXTERNAL_AGENT_DISPLAY_NAME: &str = "Claude"; +const SOURCE_EXTERNAL_AGENT_PRODUCT_NAME: &str = "Claude Code"; +const SOURCE_EXTERNAL_AGENT_UPPER_NAME: &str = "CLAUDE"; +const SOURCE_EXTERNAL_AGENT_UPPER_PRODUCT_NAME: &str = "CLAUDE-CODE"; + fn fixture_paths() -> (TempDir, PathBuf, PathBuf) { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); (root, external_agent_home, codex_home) } @@ -23,7 +31,7 @@ fn github_plugin_details() -> MigrationDetails { marketplace_name: "acme-tools".to_string(), plugin_names: vec!["formatter".to_string()], }], - sessions: Vec::new(), + ..Default::default() } } @@ -35,11 +43,14 @@ async fn detect_home_lists_config_skills_and_agents_md() { .map(|parent| parent.join(".agents").join("skills")) .unwrap_or_else(|| PathBuf::from(".agents").join("skills")); fs::create_dir_all(external_agent_home.join("skills").join("skill-a")).expect("create skills"); - fs::write(external_agent_home.join("CLAUDE.md"), "claude rules") - .expect("write external agent md"); + fs::write( + external_agent_home.join(EXTERNAL_AGENT_CONFIG_MD), + format!("{SOURCE_EXTERNAL_AGENT_NAME} rules"), + ) + .expect("write external agent md"); fs::write( external_agent_home.join("settings.json"), - r#"{"model":"claude","env":{"FOO":"bar"}}"#, + format!(r#"{{"model":"{SOURCE_EXTERNAL_AGENT_NAME}","env":{{"FOO":"bar"}}}}"#), ) .expect("write settings"); @@ -76,7 +87,7 @@ async fn detect_home_lists_config_skills_and_agents_md() { item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: format!( "Migrate {} to {}", - external_agent_home.join("CLAUDE.md").display(), + external_agent_home.join(EXTERNAL_AGENT_CONFIG_MD).display(), codex_home.join("AGENTS.md").display() ), cwd: None, @@ -134,6 +145,7 @@ async fn detect_home_lists_recent_sessions() { cwd: project_root, title: Some("first request".to_string()), }], + ..Default::default() }), }] ); @@ -146,22 +158,29 @@ async fn detect_repo_lists_agents_md_for_each_cwd() { let nested = repo_root.join("nested").join("child"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); fs::create_dir_all(&nested).expect("create nested"); - fs::write(repo_root.join("CLAUDE.md"), "Claude code guidance").expect("write source"); + fs::write( + repo_root.join(EXTERNAL_AGENT_CONFIG_MD), + format!("{SOURCE_EXTERNAL_AGENT_DISPLAY_NAME} code guidance"), + ) + .expect("write source"); - let items = service_for_paths(root.path().join(".claude"), root.path().join(".codex")) - .detect(ExternalAgentConfigDetectOptions { - include_home: false, - cwds: Some(vec![nested, repo_root.clone()]), - }) - .await - .expect("detect"); + let items = service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .detect(ExternalAgentConfigDetectOptions { + include_home: false, + cwds: Some(vec![nested, repo_root.clone()]), + }) + .await + .expect("detect"); let expected = vec![ ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: format!( "Migrate {} to {}", - repo_root.join("CLAUDE.md").display(), + repo_root.join(EXTERNAL_AGENT_CONFIG_MD).display(), repo_root.join("AGENTS.md").display(), ), cwd: Some(repo_root.clone()), @@ -171,7 +190,7 @@ async fn detect_repo_lists_agents_md_for_each_cwd() { item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: format!( "Migrate {} to {}", - repo_root.join("CLAUDE.md").display(), + repo_root.join(EXTERNAL_AGENT_CONFIG_MD).display(), repo_root.join("AGENTS.md").display(), ), cwd: Some(repo_root), @@ -188,32 +207,41 @@ async fn detect_repo_still_reports_non_plugin_items_when_home_config_is_invalid( let repo_root = root.path().join("repo"); let codex_home = root.path().join(".codex"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude").join("skills").join("skill-a")) - .expect("create repo skills"); + fs::create_dir_all( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("skills") + .join("skill-a"), + ) + .expect("create repo skills"); fs::create_dir_all(&codex_home).expect("create codex home"); fs::write(codex_home.join("config.toml"), "this is not valid = [toml") .expect("write invalid codex config"); fs::write( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{"env":{"FOO":"bar"}}"#, ) .expect("write settings"); fs::write( repo_root - .join(".claude") + .join(EXTERNAL_AGENT_DIR) .join("skills") .join("skill-a") .join("SKILL.md"), - "Use Claude Code and CLAUDE utilities.", + format!( + "Use {SOURCE_EXTERNAL_AGENT_PRODUCT_NAME} and {SOURCE_EXTERNAL_AGENT_UPPER_NAME} utilities." + ), ) .expect("write skill"); fs::write( - repo_root.join(".claude").join("CLAUDE.md"), - "Claude code guidance", + repo_root + .join(EXTERNAL_AGENT_DIR) + .join(EXTERNAL_AGENT_CONFIG_MD), + format!("{SOURCE_EXTERNAL_AGENT_DISPLAY_NAME} code guidance"), ) .expect("write agents"); - let items = service_for_paths(root.path().join(".claude"), codex_home) + let items = service_for_paths(root.path().join(EXTERNAL_AGENT_DIR), codex_home) .detect(ExternalAgentConfigDetectOptions { include_home: false, cwds: Some(vec![repo_root.clone()]), @@ -228,7 +256,10 @@ async fn detect_repo_still_reports_non_plugin_items_when_home_config_is_invalid( item_type: ExternalAgentConfigMigrationItemType::Config, description: format!( "Migrate {} into {}", - repo_root.join(".claude").join("settings.json").display(), + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("settings.json") + .display(), repo_root.join(".codex").join("config.toml").display() ), cwd: Some(repo_root.clone()), @@ -238,7 +269,7 @@ async fn detect_repo_still_reports_non_plugin_items_when_home_config_is_invalid( item_type: ExternalAgentConfigMigrationItemType::Skills, description: format!( "Migrate skills from {} to {}", - repo_root.join(".claude").join("skills").display(), + repo_root.join(EXTERNAL_AGENT_DIR).join("skills").display(), repo_root.join(".agents").join("skills").display() ), cwd: Some(repo_root.clone()), @@ -248,7 +279,10 @@ async fn detect_repo_still_reports_non_plugin_items_when_home_config_is_invalid( item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: format!( "Migrate {} to {}", - repo_root.join(".claude").join("CLAUDE.md").display(), + repo_root + .join(EXTERNAL_AGENT_DIR) + .join(EXTERNAL_AGENT_CONFIG_MD) + .display(), repo_root.join("AGENTS.md").display(), ), cwd: Some(repo_root), @@ -258,6 +292,352 @@ async fn detect_repo_still_reports_non_plugin_items_when_home_config_is_invalid( ); } +#[tokio::test] +async fn detect_repo_lists_mcp_hooks_commands_and_subagents() { + 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 dir"); + fs::create_dir_all( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("commands") + .join("pr"), + ) + .expect("create commands"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR).join("agents")).expect("create agents"); + fs::write( + repo_root.join(".mcp.json"), + r#"{"mcpServers":{"docs":{"command":"docs-server"}}}"#, + ) + .expect("write mcp"); + fs::write( + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), + r#"{"hooks":{"PreToolUse":[{"matcher":"Bash","hooks":[{"type":"command","command":"echo external-agent","timeout":3},{"type":"http","url":"https://example.invalid/hook"}]}]}}"#, + ) + .expect("write hooks"); + fs::write( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("commands") + .join("pr") + .join("review.md"), + "---\ndescription: Review PR\n---\nReview the pull request carefully.\n", + ) + .expect("write command"); + fs::write( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("agents") + .join("researcher.md"), + "---\nname: researcher\ndescription: Research role\n---\nResearch carefully.\n", + ) + .expect("write subagent"); + + let items = service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .detect(ExternalAgentConfigDetectOptions { + include_home: false, + cwds: Some(vec![repo_root.clone()]), + }) + .await + .expect("detect"); + + assert_eq!( + items, + vec![ + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, + description: format!( + "Migrate MCP servers from {} into {}", + repo_root.display(), + repo_root.join(".codex").join("config.toml").display() + ), + cwd: Some(repo_root.clone()), + details: Some(MigrationDetails { + mcp_servers: vec![NamedMigration { + name: "docs".to_string(), + }], + ..Default::default() + }), + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Hooks, + description: format!( + "Migrate hooks from {} to {}", + repo_root.join(EXTERNAL_AGENT_DIR).display(), + repo_root.join(".codex").join("hooks.json").display() + ), + cwd: Some(repo_root.clone()), + details: Some(MigrationDetails { + hooks: vec![NamedMigration { + name: "PreToolUse".to_string(), + }], + ..Default::default() + }), + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Commands, + description: format!( + "Migrate commands from {} to {}", + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("commands") + .display(), + repo_root.join(".agents").join("skills").display() + ), + cwd: Some(repo_root.clone()), + details: Some(MigrationDetails { + commands: vec![NamedMigration { + name: "source-command-pr-review".to_string(), + }], + ..Default::default() + }), + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Subagents, + description: format!( + "Migrate subagents from {} to {}", + repo_root.join(EXTERNAL_AGENT_DIR).join("agents").display(), + repo_root.join(".codex").join("agents").display() + ), + cwd: Some(repo_root), + details: Some(MigrationDetails { + subagents: vec![NamedMigration { + name: "researcher".to_string(), + }], + ..Default::default() + }), + }, + ] + ); +} + +#[tokio::test] +async fn detect_repo_skips_hooks_when_only_unsupported_hooks_exist() { + 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 dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create external agent dir"); + fs::write( + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), + r#"{"hooks":{"PreToolUse":[{"matcher":"Bash","hooks":[{"type":"command","if":"Bash(rm *)","command":"echo blocked"}]}],"SubagentStart":[{"matcher":"worker","hooks":[{"type":"command","command":"echo started"}]}]}}"#, + ) + .expect("write hooks"); + + let items = service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .detect(ExternalAgentConfigDetectOptions { + include_home: false, + cwds: Some(vec![repo_root]), + }) + .await + .expect("detect"); + + assert_eq!(items, Vec::::new()); +} + +#[tokio::test] +async fn import_repo_migrates_mcp_hooks_commands_and_subagents() { + 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 dir"); + fs::create_dir_all( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("commands") + .join("pr"), + ) + .expect("create commands"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR).join("agents")).expect("create agents"); + fs::write( + repo_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "docs": { + "command": "docs-server", + "args": ["--stdio"], + "headers": {"X-Ignored": "unsupported for stdio"}, + "env": {"DOCS_TOKEN": "${DOCS_TOKEN}", "STATIC": "yes"} + }, + "api": { + "url": "https://example.com/mcp", + "args": ["ignored-for-http"], + "env": {"IGNORED": "unsupported for http"}, + "headers": { + "Authorization": "Bearer ${API_TOKEN}", + "X-Team": "${TEAM}" + } + } + } + }"#, + ) + .expect("write mcp"); + fs::write( + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), + r#"{"hooks":{"PreToolUse":[{"matcher":"Bash","hooks":[{"type":"command","command":"echo external-agent","timeout":3},{"type":"prompt","prompt":"skip"}]}],"Stop":[{"matcher":"ignored","hooks":[{"command":"echo done"}]}]}}"#, + ) + .expect("write hooks"); + fs::write( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("commands") + .join("pr") + .join("review.md"), + "---\ndescription: Review PR\n---\nReview the pull request carefully.\n", + ) + .expect("write command"); + fs::write( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("agents") + .join("researcher.md"), + format!("---\nname: researcher\ndescription: Research role\npermissionMode: acceptEdits\nskills: [deep-research]\ntools: Bash, Read\ndisallowedTools: WebFetch\neffort: high\n---\nResearch with {SOURCE_EXTERNAL_AGENT_PRODUCT_NAME} carefully.\n"), + ) + .expect("write subagent"); + + service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .import(vec![ + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Hooks, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Commands, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Subagents, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }, + ]) + .await + .expect("import"); + + let config: TomlValue = toml::from_str( + &fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"), + ) + .expect("parse config"); + let expected_config: TomlValue = toml::from_str( + r#" +[mcp_servers.api] +url = "https://example.com/mcp" +bearer_token_env_var = "API_TOKEN" + +[mcp_servers.api.env_http_headers] +X-Team = "TEAM" + +[mcp_servers.docs] +command = "docs-server" +args = ["--stdio"] +env_vars = ["DOCS_TOKEN"] + +[mcp_servers.docs.env] +STATIC = "yes" +"#, + ) + .expect("parse expected config"); + assert_eq!(config, expected_config); + let mcp_servers = config + .get("mcp_servers") + .cloned() + .ok_or_else(|| io::Error::other("missing mcp_servers")) + .expect("mcp servers"); + let _supported_mcp_config: std::collections::HashMap< + String, + codex_config::types::McpServerConfig, + > = mcp_servers + .try_into() + .expect("migrated MCP config should be supported"); + + let hooks: JsonValue = serde_json::from_str( + &fs::read_to_string(repo_root.join(".codex").join("hooks.json")).expect("read hooks"), + ) + .expect("parse hooks"); + let _supported_hooks: codex_config::HooksFile = + serde_json::from_value(hooks.clone()).expect("migrated hooks should be supported"); + assert_eq!( + hooks, + serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [{ + "type": "command", + "command": "echo external-agent", + "timeout": 3 + }] + }], + "Stop": [{ + "hooks": [{ + "type": "command", + "command": "echo done" + }] + }] + } + }) + ); + assert!( + !repo_root + .join(".codex") + .join("hooks.migration-notes.md") + .exists() + ); + + assert_eq!( + fs::read_to_string( + repo_root + .join(".agents") + .join("skills") + .join("source-command-pr-review") + .join("SKILL.md") + ) + .expect("read command skill"), + "---\nname: \"source-command-pr-review\"\ndescription: \"Review PR\"\n---\n\n# source-command-pr-review\n\nUse this skill when the user asks to run the migrated source command `pr-review`.\n\n## Command Template\n\nReview the pull request carefully.\n" + ); + + let agent: TomlValue = toml::from_str( + &fs::read_to_string( + repo_root + .join(".codex") + .join("agents") + .join("researcher.toml"), + ) + .expect("read agent"), + ) + .expect("parse agent"); + let expected_agent: TomlValue = toml::from_str( + r#" +name = "researcher" +description = "Research role" +model_reasoning_effort = "high" +sandbox_mode = "workspace-write" +developer_instructions = """ +Research with Codex carefully.""" +"#, + ) + .expect("parse expected agent"); + assert_eq!(agent, expected_agent); +} + #[tokio::test] async fn import_home_migrates_supported_config_fields_skills_and_agents_md() { let (_root, external_agent_home, codex_home) = fixture_paths(); @@ -268,7 +648,7 @@ async fn import_home_migrates_supported_config_fields_skills_and_agents_md() { fs::create_dir_all(external_agent_home.join("skills").join("skill-a")).expect("create skills"); fs::write( external_agent_home.join("settings.json"), - r#"{"model":"claude","permissions":{"ask":["git push"]},"env":{"FOO":"bar","CI":false,"MAX_RETRIES":3,"MY_TEAM":"codex","IGNORED":null,"LIST":["a","b"],"MAP":{"x":1}},"sandbox":{"enabled":true,"network":{"allowLocalBinding":true}}}"#, + format!(r#"{{"model":"{SOURCE_EXTERNAL_AGENT_NAME}","permissions":{{"ask":["git push"]}},"env":{{"FOO":"bar","CI":false,"MAX_RETRIES":3,"MY_TEAM":"codex","IGNORED":null,"LIST":["a","b"],"MAP":{{"x":1}}}},"sandbox":{{"enabled":true,"network":{{"allowLocalBinding":true}}}}}}"#), ) .expect("write settings"); fs::write( @@ -276,12 +656,14 @@ async fn import_home_migrates_supported_config_fields_skills_and_agents_md() { .join("skills") .join("skill-a") .join("SKILL.md"), - "Use Claude Code and CLAUDE utilities.", + format!( + "Use {SOURCE_EXTERNAL_AGENT_PRODUCT_NAME} and {SOURCE_EXTERNAL_AGENT_UPPER_NAME} utilities." + ), ) .expect("write skill"); fs::write( - external_agent_home.join("CLAUDE.md"), - "Claude code guidance", + external_agent_home.join(EXTERNAL_AGENT_CONFIG_MD), + format!("{SOURCE_EXTERNAL_AGENT_DISPLAY_NAME} code guidance"), ) .expect("write agents"); @@ -331,7 +713,7 @@ async fn import_home_skips_empty_config_migration() { fs::create_dir_all(&external_agent_home).expect("create external agent home"); fs::write( external_agent_home.join("settings.json"), - r#"{"model":"claude","sandbox":{"enabled":false}}"#, + format!(r#"{{"model":"{SOURCE_EXTERNAL_AGENT_NAME}","sandbox":{{"enabled":false}}}}"#), ) .expect("write settings"); @@ -353,7 +735,7 @@ 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")) + fs::create_dir_all(marketplace_root.join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR)) .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"); @@ -376,7 +758,7 @@ async fn import_local_plugins_returns_completed_status() { .expect("write settings"); fs::write( marketplace_root - .join(".claude-plugin") + .join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR) .join("marketplace.json"), r#"{ "name": "my-plugins", @@ -405,7 +787,7 @@ async fn import_local_plugins_returns_completed_status() { marketplace_name: "my-plugins".to_string(), plugin_names: vec!["cloudflare".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }]) .await @@ -446,7 +828,7 @@ async fn import_git_plugins_returns_pending_async_status() { marketplace_name: "acme-tools".to_string(), plugin_names: vec!["formatter".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }]) .await @@ -461,7 +843,7 @@ async fn import_git_plugins_returns_pending_async_status() { marketplace_name: "acme-tools".to_string(), plugin_names: vec!["formatter".to_string()], }], - sessions: Vec::new(), + ..Default::default() }, }] ); @@ -532,34 +914,43 @@ async fn import_repo_agents_md_rewrites_terms_and_skips_non_empty_targets() { fs::create_dir_all(repo_root.join(".git")).expect("create git"); fs::create_dir_all(repo_with_existing_target.join(".git")).expect("create git"); fs::write( - repo_root.join("CLAUDE.md"), - "Claude code\nclaude\nCLAUDE-CODE\nSee CLAUDE.md\n", + repo_root.join(EXTERNAL_AGENT_CONFIG_MD), + format!( + "{SOURCE_EXTERNAL_AGENT_PRODUCT_NAME}\n{SOURCE_EXTERNAL_AGENT_NAME}\n{SOURCE_EXTERNAL_AGENT_UPPER_PRODUCT_NAME}\nSee {EXTERNAL_AGENT_CONFIG_MD}\n" + ), + ) + .expect("write source"); + fs::write( + repo_with_existing_target.join(EXTERNAL_AGENT_CONFIG_MD), + "new source", ) .expect("write source"); - fs::write(repo_with_existing_target.join("CLAUDE.md"), "new source").expect("write source"); fs::write( repo_with_existing_target.join("AGENTS.md"), "keep existing target", ) .expect("write target"); - service_for_paths(root.path().join(".claude"), root.path().join(".codex")) - .import(vec![ - ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::AgentsMd, - description: String::new(), - cwd: Some(repo_root.clone()), - details: None, - }, - ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::AgentsMd, - description: String::new(), - cwd: Some(repo_with_existing_target.clone()), - details: None, - }, - ]) - .await - .expect("import"); + service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .import(vec![ + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }, + ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: String::new(), + cwd: Some(repo_with_existing_target.clone()), + details: None, + }, + ]) + .await + .expect("import"); assert_eq!( fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"), @@ -577,18 +968,25 @@ async fn import_repo_agents_md_overwrites_empty_targets() { 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("CLAUDE.md"), "Claude code guidance").expect("write source"); + fs::write( + repo_root.join(EXTERNAL_AGENT_CONFIG_MD), + format!("{SOURCE_EXTERNAL_AGENT_DISPLAY_NAME} code guidance"), + ) + .expect("write source"); fs::write(repo_root.join("AGENTS.md"), " \n\t").expect("write empty target"); - service_for_paths(root.path().join(".claude"), root.path().join(".codex")) - .import(vec![ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::AgentsMd, - description: String::new(), - cwd: Some(repo_root.clone()), - details: None, - }]) - .await - .expect("import"); + service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }]) + .await + .expect("import"); assert_eq!( fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"), @@ -601,21 +999,26 @@ async fn detect_repo_prefers_non_empty_external_agent_agents_source() { 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::create_dir_all(repo_root.join(".claude")).expect("create dot claude"); - fs::write(repo_root.join("CLAUDE.md"), " \n\t").expect("write empty root source"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create external agent dir"); + fs::write(repo_root.join(EXTERNAL_AGENT_CONFIG_MD), " \n\t").expect("write empty root source"); fs::write( - repo_root.join(".claude").join("CLAUDE.md"), - "Claude code guidance", + repo_root + .join(EXTERNAL_AGENT_DIR) + .join(EXTERNAL_AGENT_CONFIG_MD), + format!("{SOURCE_EXTERNAL_AGENT_DISPLAY_NAME} code guidance"), ) - .expect("write dot claude source"); + .expect("write external agent source"); - let items = service_for_paths(root.path().join(".claude"), root.path().join(".codex")) - .detect(ExternalAgentConfigDetectOptions { - include_home: false, - cwds: Some(vec![repo_root.clone()]), - }) - .await - .expect("detect"); + let items = service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .detect(ExternalAgentConfigDetectOptions { + include_home: false, + cwds: Some(vec![repo_root.clone()]), + }) + .await + .expect("detect"); assert_eq!( items, @@ -623,7 +1026,10 @@ async fn detect_repo_prefers_non_empty_external_agent_agents_source() { item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: format!( "Migrate {} to {}", - repo_root.join(".claude").join("CLAUDE.md").display(), + repo_root + .join(EXTERNAL_AGENT_DIR) + .join(EXTERNAL_AGENT_CONFIG_MD) + .display(), repo_root.join("AGENTS.md").display(), ), cwd: Some(repo_root), @@ -633,21 +1039,90 @@ async fn detect_repo_prefers_non_empty_external_agent_agents_source() { } #[tokio::test] -async fn import_repo_uses_non_empty_external_agent_agents_source() { +async fn import_repo_hooks_preserves_disabled_codex_hooks_feature() { 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::create_dir_all(repo_root.join(".claude")).expect("create dot claude"); - fs::write(repo_root.join("CLAUDE.md"), "").expect("write empty root source"); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create external agent dir"); + fs::create_dir_all(repo_root.join(".codex")).expect("create codex dir"); fs::write( - repo_root.join(".claude").join("CLAUDE.md"), - "Claude code guidance", + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), + r#"{"hooks":{"Stop":[{"hooks":[{"command":"echo done"}]}]}}"#, ) - .expect("write dot claude source"); + .expect("write hooks"); + fs::write( + repo_root.join(".codex").join("config.toml"), + "[features]\ncodex_hooks = false\n", + ) + .expect("write config"); - service_for_paths(root.path().join(".claude"), root.path().join(".codex")) + service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Hooks, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }]) + .await + .expect("import"); + + assert_eq!( + fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"), + "[features]\ncodex_hooks = false\n" + ); + let hooks: JsonValue = serde_json::from_str( + &fs::read_to_string(repo_root.join(".codex").join("hooks.json")).expect("read hooks"), + ) + .expect("parse hooks"); + assert_eq!( + hooks, + serde_json::json!({ + "hooks": { + "Stop": [{ + "hooks": [{ + "type": "command", + "command": "echo done" + }] + }] + } + }) + ); +} + +#[tokio::test] +async fn import_repo_mcp_uses_home_settings_toggles_when_repo_settings_missing() { + let root = TempDir::new().expect("create tempdir"); + let repo_root = root.path().join("repo"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + external_agent_home.join("settings.json"), + r#"{"disabledMcpjsonServers":["blocked"]}"#, + ) + .expect("write home settings"); + fs::write( + root.path().join(EXTERNAL_AGENT_PROJECT_CONFIG_FILE), + serde_json::json!({ + "projects": { + repo_root.display().to_string(): { + "mcpServers": { + "allowed": {"command": "allowed-server"}, + "blocked": {"command": "blocked-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + service_for_paths(external_agent_home, root.path().join(".codex")) .import(vec![ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, description: String::new(), cwd: Some(repo_root.clone()), details: None, @@ -655,6 +1130,96 @@ async fn import_repo_uses_non_empty_external_agent_agents_source() { .await .expect("import"); + let config: TomlValue = toml::from_str( + &fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"), + ) + .expect("parse config"); + let expected: TomlValue = toml::from_str( + r#" +[mcp_servers.allowed] +command = "allowed-server" +"#, + ) + .expect("parse expected config"); + assert_eq!(config, expected); +} + +#[tokio::test] +async fn import_repo_mcp_ignores_invalid_home_settings_when_repo_settings_missing() { + let root = TempDir::new().expect("create tempdir"); + let repo_root = root.path().join("repo"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write(external_agent_home.join("settings.json"), "{ invalid json") + .expect("write invalid home settings"); + fs::write( + root.path().join(EXTERNAL_AGENT_PROJECT_CONFIG_FILE), + serde_json::json!({ + "projects": { + repo_root.display().to_string(): { + "mcpServers": { + "docs": {"command": "docs-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + service_for_paths(external_agent_home, root.path().join(".codex")) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }]) + .await + .expect("import"); + + let config: TomlValue = toml::from_str( + &fs::read_to_string(repo_root.join(".codex").join("config.toml")).expect("read config"), + ) + .expect("parse config"); + let expected: TomlValue = toml::from_str( + r#" +[mcp_servers.docs] +command = "docs-server" +"#, + ) + .expect("parse expected config"); + assert_eq!(config, expected); +} + +#[tokio::test] +async fn import_repo_uses_non_empty_external_agent_agents_source() { + 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::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create external agent dir"); + fs::write(repo_root.join(EXTERNAL_AGENT_CONFIG_MD), "").expect("write empty root source"); + fs::write( + repo_root + .join(EXTERNAL_AGENT_DIR) + .join(EXTERNAL_AGENT_CONFIG_MD), + format!("{SOURCE_EXTERNAL_AGENT_DISPLAY_NAME} code guidance"), + ) + .expect("write external agent source"); + + service_for_paths( + root.path().join(EXTERNAL_AGENT_DIR), + root.path().join(".codex"), + ) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::AgentsMd, + description: String::new(), + cwd: Some(repo_root.clone()), + details: None, + }]) + .await + .expect("import"); + assert_eq!( fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"), "Codex guidance" @@ -686,7 +1251,7 @@ async fn detect_home_lists_enabled_plugins_from_settings() { }, "extraKnownMarketplaces": { "acme-tools": { - "source": "acme-corp/claude-plugins" + "source": "acme-corp/external-agent-plugins" } } }"#, @@ -715,7 +1280,7 @@ async fn detect_home_lists_enabled_plugins_from_settings() { marketplace_name: "acme-tools".to_string(), plugin_names: vec!["deployer".to_string(), "formatter".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }] ); @@ -724,14 +1289,14 @@ async fn detect_home_lists_enabled_plugins_from_settings() { #[tokio::test] async fn detect_repo_skips_plugins_that_are_already_configured_in_codex() { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); let repo_root = root.path().join("repo"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude")).expect("create repo external agent dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create repo external agent dir"); fs::create_dir_all(&codex_home).expect("create codex home"); fs::write( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{ "enabledPlugins": { "formatter@acme-tools": true, @@ -739,7 +1304,7 @@ async fn detect_repo_skips_plugins_that_are_already_configured_in_codex() { }, "extraKnownMarketplaces": { "acme-tools": { - "source": "acme-corp/claude-plugins" + "source": "acme-corp/external-agent-plugins" } } }"#, @@ -768,7 +1333,10 @@ enabled = true item_type: ExternalAgentConfigMigrationItemType::Plugins, description: format!( "Migrate enabled plugins from {}", - repo_root.join(".claude").join("settings.json").display() + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("settings.json") + .display() ), cwd: Some(repo_root), details: Some(MigrationDetails { @@ -776,7 +1344,7 @@ enabled = true marketplace_name: "acme-tools".to_string(), plugin_names: vec!["deployer".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }] ); @@ -785,21 +1353,21 @@ enabled = true #[tokio::test] async fn detect_repo_skips_plugins_that_are_disabled_in_codex() { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); let repo_root = root.path().join("repo"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude")).expect("create repo external agent dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create repo external agent dir"); fs::create_dir_all(&codex_home).expect("create codex home"); fs::write( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{ "enabledPlugins": { "formatter@acme-tools": true }, "extraKnownMarketplaces": { "acme-tools": { - "source": "acme-corp/claude-plugins" + "source": "acme-corp/external-agent-plugins" } } }"#, @@ -828,21 +1396,21 @@ enabled = false #[tokio::test] async fn detect_repo_skips_plugins_without_explicit_enabled_in_codex() { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); let repo_root = root.path().join("repo"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude")).expect("create repo external agent dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create repo external agent dir"); fs::create_dir_all(&codex_home).expect("create codex home"); fs::write( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{ "enabledPlugins": { "formatter@acme-tools": true }, "extraKnownMarketplaces": { "acme-tools": { - "source": "acme-corp/claude-plugins" + "source": "acme-corp/external-agent-plugins" } } }"#, @@ -883,22 +1451,22 @@ async fn import_plugins_requires_details() { #[tokio::test] async fn detect_repo_does_not_skip_plugins_only_configured_in_project_codex() { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); let repo_root = root.path().join("repo"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude")).expect("create repo external agent dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create repo external agent dir"); fs::create_dir_all(repo_root.join(".codex")).expect("create repo codex dir"); fs::create_dir_all(&codex_home).expect("create codex home"); fs::write( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{ "enabledPlugins": { "formatter@acme-tools": true }, "extraKnownMarketplaces": { "acme-tools": { - "source": "acme-corp/claude-plugins" + "source": "acme-corp/external-agent-plugins" } } }"#, @@ -927,7 +1495,10 @@ enabled = true item_type: ExternalAgentConfigMigrationItemType::Plugins, description: format!( "Migrate enabled plugins from {}", - repo_root.join(".claude").join("settings.json").display() + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("settings.json") + .display() ), cwd: Some(repo_root), details: Some(MigrationDetails { @@ -935,7 +1506,7 @@ enabled = true marketplace_name: "acme-tools".to_string(), plugin_names: vec!["formatter".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }] ); @@ -999,12 +1570,12 @@ async fn detect_home_skips_plugins_with_invalid_marketplace_source() { #[tokio::test] async fn detect_repo_filters_plugins_against_installed_marketplace() { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); let repo_root = root.path().join("repo"); let marketplace_root = codex_home.join(".tmp").join("marketplaces").join("debug"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude")).expect("create repo external agent dir"); + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create repo external agent dir"); fs::create_dir_all(marketplace_root.join(".agents").join("plugins")) .expect("create marketplace manifest dir"); fs::create_dir_all( @@ -1022,7 +1593,7 @@ async fn detect_repo_filters_plugins_against_installed_marketplace() { ) .expect("create available plugin"); fs::write( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{ "enabledPlugins": { "sample@debug": true, @@ -1108,7 +1679,10 @@ source = "owner/debug-marketplace" item_type: ExternalAgentConfigMigrationItemType::Plugins, description: format!( "Migrate enabled plugins from {}", - repo_root.join(".claude").join("settings.json").display() + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("settings.json") + .display() ), cwd: Some(repo_root), details: Some(MigrationDetails { @@ -1116,7 +1690,7 @@ source = "owner/debug-marketplace" marketplace_name: "debug".to_string(), plugin_names: vec!["available".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }] ); @@ -1135,7 +1709,7 @@ async fn import_plugins_requires_source_marketplace_details() { "extraKnownMarketplaces": { "acme-tools": { "source": "github", - "repo": "acme-corp/claude-plugins" + "repo": "acme-corp/external-agent-plugins" } } }"#, @@ -1150,7 +1724,7 @@ async fn import_plugins_requires_source_marketplace_details() { marketplace_name: "other-tools".to_string(), plugin_names: github_plugin_details().plugins[0].plugin_names.clone(), }], - sessions: Vec::new(), + ..Default::default() }), ) .await @@ -1208,7 +1782,7 @@ async fn import_plugins_supports_external_agent_plugin_marketplace_layout() { 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")) + fs::create_dir_all(marketplace_root.join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR)) .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"); @@ -1231,7 +1805,7 @@ async fn import_plugins_supports_external_agent_plugin_marketplace_layout() { .expect("write settings"); fs::write( marketplace_root - .join(".claude-plugin") + .join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR) .join("marketplace.json"), r#"{ "name": "my-plugins", @@ -1258,7 +1832,7 @@ async fn import_plugins_supports_external_agent_plugin_marketplace_layout() { marketplace_name: "my-plugins".to_string(), plugin_names: vec!["cloudflare".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), ) .await @@ -1283,7 +1857,7 @@ async fn detect_home_supports_relative_external_agent_plugin_marketplace_path() 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")) + fs::create_dir_all(marketplace_root.join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR)) .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"); @@ -1305,7 +1879,7 @@ async fn detect_home_supports_relative_external_agent_plugin_marketplace_path() .expect("write settings"); fs::write( marketplace_root - .join(".claude-plugin") + .join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR) .join("marketplace.json"), r#"{ "name": "my-plugins", @@ -1346,25 +1920,27 @@ async fn detect_home_supports_relative_external_agent_plugin_marketplace_path() marketplace_name: "my-plugins".to_string(), plugin_names: vec!["cloudflare".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }] ); } #[tokio::test] -async fn detect_home_infers_claude_official_marketplace_when_missing_from_settings() { +async fn detect_home_infers_external_official_marketplace_when_missing_from_settings() { let (_root, external_agent_home, codex_home) = fixture_paths(); fs::create_dir_all(&external_agent_home).expect("create external agent home"); fs::create_dir_all(&codex_home).expect("create codex home"); fs::write( external_agent_home.join("settings.json"), - r#"{ - "enabledPlugins": { - "sample@claude-plugins-official": true - } - }"#, + format!( + r#"{{ + "enabledPlugins": {{ + "sample@{EXTERNAL_OFFICIAL_MARKETPLACE_NAME}": true + }} + }}"# + ), ) .expect("write settings"); @@ -1387,10 +1963,10 @@ async fn detect_home_infers_claude_official_marketplace_when_missing_from_settin cwd: None, details: Some(MigrationDetails { plugins: vec![PluginsMigration { - marketplace_name: "claude-plugins-official".to_string(), + marketplace_name: EXTERNAL_OFFICIAL_MARKETPLACE_NAME.to_string(), plugin_names: vec!["sample".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }] ); @@ -1401,7 +1977,7 @@ async fn import_plugins_supports_relative_external_agent_plugin_marketplace_path 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")) + fs::create_dir_all(marketplace_root.join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR)) .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"); @@ -1423,7 +1999,7 @@ async fn import_plugins_supports_relative_external_agent_plugin_marketplace_path .expect("write settings"); fs::write( marketplace_root - .join(".claude-plugin") + .join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR) .join("marketplace.json"), r#"{ "name": "my-plugins", @@ -1450,7 +2026,7 @@ async fn import_plugins_supports_relative_external_agent_plugin_marketplace_path marketplace_name: "my-plugins".to_string(), plugin_names: vec!["cloudflare".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), ) .await @@ -1471,18 +2047,20 @@ async fn import_plugins_supports_relative_external_agent_plugin_marketplace_path } #[tokio::test] -async fn import_plugins_infers_claude_official_marketplace_when_missing_from_settings() { +async fn import_plugins_infers_external_official_marketplace_when_missing_from_settings() { let (_root, external_agent_home, codex_home) = fixture_paths(); fs::create_dir_all(&external_agent_home).expect("create external agent home"); fs::create_dir_all(&codex_home).expect("create codex home"); fs::write( external_agent_home.join("settings.json"), - r#"{ - "enabledPlugins": { - "sample@claude-plugins-official": true - } - }"#, + format!( + r#"{{ + "enabledPlugins": {{ + "sample@{EXTERNAL_OFFICIAL_MARKETPLACE_NAME}": true + }} + }}"# + ), ) .expect("write settings"); @@ -1491,10 +2069,10 @@ async fn import_plugins_infers_claude_official_marketplace_when_missing_from_set /*cwd*/ None, Some(MigrationDetails { plugins: vec![PluginsMigration { - marketplace_name: "claude-plugins-official".to_string(), + marketplace_name: EXTERNAL_OFFICIAL_MARKETPLACE_NAME.to_string(), plugin_names: vec!["sample".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), ) .await @@ -1503,10 +2081,10 @@ async fn import_plugins_infers_claude_official_marketplace_when_missing_from_set assert_eq!( outcome, PluginImportOutcome { - succeeded_marketplaces: vec!["claude-plugins-official".to_string()], + succeeded_marketplaces: vec![EXTERNAL_OFFICIAL_MARKETPLACE_NAME.to_string()], succeeded_plugin_ids: Vec::new(), failed_marketplaces: Vec::new(), - failed_plugin_ids: vec!["sample@claude-plugins-official".to_string()], + failed_plugin_ids: vec![format!("sample@{EXTERNAL_OFFICIAL_MARKETPLACE_NAME}")], } ); } @@ -1514,20 +2092,20 @@ async fn import_plugins_infers_claude_official_marketplace_when_missing_from_set #[tokio::test] async fn detect_repo_supports_project_relative_external_agent_plugin_marketplace_path() { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); let repo_root = root.path().join("repo"); let marketplace_root = repo_root.join("my-marketplace"); let plugin_root = marketplace_root.join("plugins").join("cloudflare"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude")).expect("create repo external agent dir"); - fs::create_dir_all(marketplace_root.join(".claude-plugin")) + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create repo external agent dir"); + fs::create_dir_all(marketplace_root.join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR)) .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( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{ "enabledPlugins": { "cloudflare@my-plugins": true @@ -1543,7 +2121,7 @@ async fn detect_repo_supports_project_relative_external_agent_plugin_marketplace .expect("write settings"); fs::write( marketplace_root - .join(".claude-plugin") + .join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR) .join("marketplace.json"), r#"{ "name": "my-plugins", @@ -1576,7 +2154,10 @@ async fn detect_repo_supports_project_relative_external_agent_plugin_marketplace item_type: ExternalAgentConfigMigrationItemType::Plugins, description: format!( "Migrate enabled plugins from {}", - repo_root.join(".claude").join("settings.json").display() + repo_root + .join(EXTERNAL_AGENT_DIR) + .join("settings.json") + .display() ), cwd: Some(repo_root), details: Some(MigrationDetails { @@ -1584,7 +2165,7 @@ async fn detect_repo_supports_project_relative_external_agent_plugin_marketplace marketplace_name: "my-plugins".to_string(), plugin_names: vec!["cloudflare".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), }] ); @@ -1593,20 +2174,20 @@ async fn detect_repo_supports_project_relative_external_agent_plugin_marketplace #[tokio::test] async fn import_plugins_supports_project_relative_external_agent_plugin_marketplace_path() { let root = TempDir::new().expect("create tempdir"); - let external_agent_home = root.path().join(".claude"); + let external_agent_home = root.path().join(EXTERNAL_AGENT_DIR); let codex_home = root.path().join(".codex"); let repo_root = root.path().join("repo"); let marketplace_root = repo_root.join("my-marketplace"); let plugin_root = marketplace_root.join("plugins").join("cloudflare"); fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); - fs::create_dir_all(repo_root.join(".claude")).expect("create repo external agent dir"); - fs::create_dir_all(marketplace_root.join(".claude-plugin")) + fs::create_dir_all(repo_root.join(EXTERNAL_AGENT_DIR)).expect("create repo external agent dir"); + fs::create_dir_all(marketplace_root.join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR)) .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( - repo_root.join(".claude").join("settings.json"), + repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), r#"{ "enabledPlugins": { "cloudflare@my-plugins": true @@ -1622,7 +2203,7 @@ async fn import_plugins_supports_project_relative_external_agent_plugin_marketpl .expect("write settings"); fs::write( marketplace_root - .join(".claude-plugin") + .join(EXTERNAL_AGENT_PLUGIN_MANIFEST_DIR) .join("marketplace.json"), r#"{ "name": "my-plugins", @@ -1649,7 +2230,7 @@ async fn import_plugins_supports_project_relative_external_agent_plugin_marketpl marketplace_name: "my-plugins".to_string(), plugin_names: vec!["cloudflare".to_string()], }], - sessions: Vec::new(), + ..Default::default() }), ) .await 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 30d4cc8f4..f6c6c2f30 100644 --- a/codex-rs/app-server/src/external_agent_config_api.rs +++ b/codex-rs/app-server/src/external_agent_config_api.rs @@ -2,17 +2,22 @@ use crate::config::external_agent_config::ExternalAgentConfigDetectOptions; 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::error_code::internal_error; use crate::error_code::invalid_params; +use codex_app_server_protocol::CommandMigration; use codex_app_server_protocol::ExternalAgentConfigDetectParams; use codex_app_server_protocol::ExternalAgentConfigDetectResponse; use codex_app_server_protocol::ExternalAgentConfigImportParams; use codex_app_server_protocol::ExternalAgentConfigMigrationItem; use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; +use codex_app_server_protocol::HookMigration; use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::McpServerMigration; use codex_app_server_protocol::MigrationDetails; use codex_app_server_protocol::PluginsMigration; +use codex_app_server_protocol::SubagentMigration; use codex_external_agent_sessions::ExternalAgentSessionMigration as CoreSessionMigration; use codex_external_agent_sessions::PendingSessionImport; use codex_external_agent_sessions::PrepareSessionImportsError; @@ -68,6 +73,13 @@ impl ExternalAgentConfigApi { CoreMigrationItemType::McpServerConfig => { ExternalAgentConfigMigrationItemType::McpServerConfig } + CoreMigrationItemType::Subagents => { + ExternalAgentConfigMigrationItemType::Subagents + } + CoreMigrationItemType::Hooks => ExternalAgentConfigMigrationItemType::Hooks, + CoreMigrationItemType::Commands => { + ExternalAgentConfigMigrationItemType::Commands + } CoreMigrationItemType::Sessions => { ExternalAgentConfigMigrationItemType::Sessions } @@ -92,6 +104,30 @@ impl ExternalAgentConfigApi { title: session.title, }) .collect(), + mcp_servers: details + .mcp_servers + .into_iter() + .map(|mcp_server| McpServerMigration { + name: mcp_server.name, + }) + .collect(), + hooks: details + .hooks + .into_iter() + .map(|hook| HookMigration { name: hook.name }) + .collect(), + subagents: details + .subagents + .into_iter() + .map(|subagent| SubagentMigration { + name: subagent.name, + }) + .collect(), + commands: details + .commands + .into_iter() + .map(|command| CommandMigration { name: command.name }) + .collect(), }), }) .collect(), @@ -182,6 +218,15 @@ impl ExternalAgentConfigApi { ExternalAgentConfigMigrationItemType::McpServerConfig => { CoreMigrationItemType::McpServerConfig } + ExternalAgentConfigMigrationItemType::Subagents => { + CoreMigrationItemType::Subagents + } + ExternalAgentConfigMigrationItemType::Hooks => { + CoreMigrationItemType::Hooks + } + ExternalAgentConfigMigrationItemType::Commands => { + CoreMigrationItemType::Commands + } ExternalAgentConfigMigrationItemType::Sessions => { CoreMigrationItemType::Sessions } @@ -209,6 +254,30 @@ impl ExternalAgentConfigApi { title: session.title, }) .collect(), + mcp_servers: details + .mcp_servers + .into_iter() + .map(|mcp_server| CoreNamedMigration { + name: mcp_server.name, + }) + .collect(), + hooks: details + .hooks + .into_iter() + .map(|hook| CoreNamedMigration { name: hook.name }) + .collect(), + subagents: details + .subagents + .into_iter() + .map(|subagent| CoreNamedMigration { + name: subagent.name, + }) + .collect(), + commands: details + .commands + .into_iter() + .map(|command| CoreNamedMigration { name: command.name }) + .collect(), } }), }) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index df77e8ca2..65bf22426 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -48,6 +48,7 @@ use codex_app_server_protocol::ExperimentalFeatureEnablementSetParams; use codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification; 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::InitializeResponse; use codex_app_server_protocol::JSONRPCError; @@ -1172,6 +1173,7 @@ impl MessageProcessor { request_id: ConnectionRequestId, params: ExternalAgentConfigImportParams, ) -> Result<(), JSONRPCErrorError> { + let needs_runtime_refresh = migration_items_need_runtime_refresh(¶ms.migration_items); let has_plugin_imports = params.migration_items.iter().any(|item| { matches!( item.item_type, @@ -1182,7 +1184,7 @@ impl MessageProcessor { .external_agent_config_api .prepare_pending_session_imports(¶ms)?; let pending_plugin_imports = self.external_agent_config_api.import(params).await?; - if has_plugin_imports { + if needs_runtime_refresh { self.handle_config_mutation().await; } for pending_session_import in pending_session_imports { @@ -1241,5 +1243,60 @@ impl MessageProcessor { } } +fn migration_items_need_runtime_refresh(items: &[ExternalAgentConfigMigrationItem]) -> bool { + items.iter().any(|item| { + matches!( + item.item_type, + ExternalAgentConfigMigrationItemType::Config + | ExternalAgentConfigMigrationItemType::Skills + | ExternalAgentConfigMigrationItemType::McpServerConfig + | ExternalAgentConfigMigrationItemType::Hooks + | ExternalAgentConfigMigrationItemType::Commands + | ExternalAgentConfigMigrationItemType::Plugins + ) + }) +} + #[cfg(test)] mod tracing_tests; + +#[cfg(test)] +mod tests { + use super::*; + + fn migration_item( + item_type: ExternalAgentConfigMigrationItemType, + ) -> ExternalAgentConfigMigrationItem { + ExternalAgentConfigMigrationItem { + item_type, + description: String::new(), + cwd: None, + details: None, + } + } + + #[test] + fn migration_items_that_update_runtime_sources_trigger_refresh() { + assert!(migration_items_need_runtime_refresh(&[migration_item( + ExternalAgentConfigMigrationItemType::Config, + )])); + assert!(migration_items_need_runtime_refresh(&[migration_item( + ExternalAgentConfigMigrationItemType::Skills, + )])); + assert!(migration_items_need_runtime_refresh(&[migration_item( + ExternalAgentConfigMigrationItemType::McpServerConfig, + )])); + assert!(migration_items_need_runtime_refresh(&[migration_item( + ExternalAgentConfigMigrationItemType::Hooks, + )])); + assert!(migration_items_need_runtime_refresh(&[migration_item( + ExternalAgentConfigMigrationItemType::Commands, + )])); + assert!(migration_items_need_runtime_refresh(&[migration_item( + ExternalAgentConfigMigrationItemType::Plugins, + )])); + assert!(!migration_items_need_runtime_refresh(&[migration_item( + ExternalAgentConfigMigrationItemType::Sessions, + )])); + } +} diff --git a/codex-rs/external-agent-migration/BUILD.bazel b/codex-rs/external-agent-migration/BUILD.bazel new file mode 100644 index 000000000..f0cf82950 --- /dev/null +++ b/codex-rs/external-agent-migration/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "external-agent-migration", + crate_name = "codex_external_agent_migration", +) diff --git a/codex-rs/external-agent-migration/Cargo.toml b/codex-rs/external-agent-migration/Cargo.toml new file mode 100644 index 000000000..a515b3783 --- /dev/null +++ b/codex-rs/external-agent-migration/Cargo.toml @@ -0,0 +1,23 @@ +[package] +name = "codex-external-agent-migration" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lib] +doctest = false +name = "codex_external_agent_migration" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-hooks = { workspace = true } +serde_json = { workspace = true } +serde_yaml = { workspace = true } +toml = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs new file mode 100644 index 000000000..e01c92389 --- /dev/null +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -0,0 +1,1921 @@ +//! Migration helpers for importing external-agent configuration into Codex. + +use codex_hooks::HOOK_EVENT_NAMES; +use codex_hooks::HOOK_EVENT_NAMES_WITH_MATCHERS; +use serde_json::Value as JsonValue; +use serde_yaml::Value as YamlValue; +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::fs; +use std::io; +use std::path::Path; +use std::path::PathBuf; +use toml::Value as TomlValue; + +const SOURCE_EXTERNAL_AGENT_NAME: &str = "claude"; +const EXTERNAL_AGENT_MCP_CONFIG_FILE: &str = ".mcp.json"; +const EXTERNAL_AGENT_HOOKS_SUBDIR: &str = "hooks"; +const EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR: &str = "hooks"; +const COMMAND_SKILL_PREFIX: &str = "source-command"; +const MAX_SKILL_NAME_LEN: usize = 64; +const MAX_SKILL_DESCRIPTION_LEN: usize = 1024; + +#[derive(Debug)] +struct ParsedDocument { + frontmatter: BTreeMap, + body: String, + frontmatter_error: Option, +} + +#[derive(Debug)] +enum FrontmatterValue { + Scalar(String), + Other, +} + +#[derive(Debug)] +struct AgentMetadata { + name: String, + description: String, + permission_mode: Option, + effort: Option, +} + +pub fn build_mcp_config_from_external( + source_root: &Path, + external_agent_home: Option<&Path>, + settings: Option<&JsonValue>, +) -> io::Result { + let mcp_servers = read_external_mcp_servers(source_root, external_agent_home)?; + if mcp_servers.is_empty() { + return Ok(TomlValue::Table(Default::default())); + } + + let enabled_servers = settings + .and_then(|settings| settings.get("enabledMcpjsonServers")) + .map(json_string_vec) + .unwrap_or_default(); + let disabled_servers = settings + .and_then(|settings| settings.get("disabledMcpjsonServers")) + .map(json_string_vec) + .unwrap_or_default() + .into_iter() + .collect::>(); + + let mut servers = toml::map::Map::new(); + for (server_name, server_config) in mcp_servers { + if let Some(server) = mcp_server_toml_table( + &server_name, + server_config.as_object(), + &enabled_servers, + &disabled_servers, + ) { + servers.insert(server_name.clone(), TomlValue::Table(server)); + } + } + + if servers.is_empty() { + return Ok(TomlValue::Table(Default::default())); + } + + let mut root = toml::map::Map::new(); + root.insert("mcp_servers".to_string(), TomlValue::Table(servers)); + Ok(TomlValue::Table(root)) +} + +pub fn hooks_migration_description( + source_external_agent_dir: &Path, + target_hooks: &Path, +) -> io::Result> { + if hook_migration_event_names(source_external_agent_dir, target_hooks)?.is_empty() { + return Ok(None); + } + + Ok(Some(format!( + "Migrate hooks from {} to {}", + source_external_agent_dir.display(), + target_hooks.display() + ))) +} + +pub fn hook_migration_event_names( + source_external_agent_dir: &Path, + target_hooks: &Path, +) -> io::Result> { + let migration = hook_migration(source_external_agent_dir, target_hooks.parent())?; + Ok(migration.keys().cloned().collect()) +} + +pub fn import_hooks(source_external_agent_dir: &Path, target_hooks: &Path) -> io::Result { + let Some(parent) = target_hooks.parent() else { + return Err(invalid_data_error("hooks target path has no parent")); + }; + let migration = hook_migration(source_external_agent_dir, Some(parent))?; + if migration.is_empty() { + return Ok(false); + } + + fs::create_dir_all(parent)?; + + let mut wrote_active_hooks = false; + if is_missing_or_empty_text_file(target_hooks)? { + copy_hook_scripts(source_external_agent_dir, parent)?; + let mut payload = serde_json::Map::new(); + payload.insert("hooks".to_string(), JsonValue::Object(migration)); + let rendered = serde_json::to_string_pretty(&JsonValue::Object(payload)) + .map_err(|err| invalid_data_error(format!("failed to serialize hooks.json: {err}")))?; + fs::write(target_hooks, format!("{rendered}\n"))?; + wrote_active_hooks = true; + } + + Ok(wrote_active_hooks) +} + +pub fn count_missing_subagents(source_agents: &Path, target_agents: &Path) -> io::Result { + Ok(missing_subagent_names(source_agents, target_agents)?.len()) +} + +pub fn missing_subagent_names( + source_agents: &Path, + target_agents: &Path, +) -> io::Result> { + let mut names = Vec::new(); + for source_file in agent_source_files(source_agents)? { + let document = parse_document(&source_file)?; + let Some(metadata) = agent_metadata(&document) else { + continue; + }; + let Some(target) = subagent_target_file(&source_file, target_agents) else { + continue; + }; + if !target.exists() { + names.push(metadata.name); + } + } + Ok(names) +} + +pub fn import_subagents(source_agents: &Path, target_agents: &Path) -> io::Result { + if !source_agents.is_dir() { + return Ok(0); + } + + fs::create_dir_all(target_agents)?; + let mut imported = 0usize; + for source_file in agent_source_files(source_agents)? { + let Some(target) = subagent_target_file(&source_file, target_agents) else { + continue; + }; + if target.exists() { + continue; + } + let document = parse_document(&source_file)?; + let Some(metadata) = agent_metadata(&document) else { + continue; + }; + fs::write(&target, render_agent_toml(&document.body, &metadata)?)?; + imported += 1; + } + + Ok(imported) +} + +pub fn count_missing_commands(source_commands: &Path, target_skills: &Path) -> io::Result { + Ok(missing_command_names(source_commands, target_skills)?.len()) +} + +pub fn missing_command_names( + source_commands: &Path, + target_skills: &Path, +) -> io::Result> { + Ok(unique_supported_command_sources(source_commands)? + .into_iter() + .filter(|(_source_file, name)| !target_skills.join(name).exists()) + .map(|(_source_file, name)| name) + .collect()) +} + +pub fn import_commands(source_commands: &Path, target_skills: &Path) -> io::Result { + if !source_commands.is_dir() { + return Ok(0); + } + + fs::create_dir_all(target_skills)?; + let mut imported = 0usize; + for (source_file, name) in unique_supported_command_sources(source_commands)? { + let document = parse_document(&source_file)?; + let target_dir = target_skills.join(&name); + if target_dir.exists() { + continue; + } + fs::create_dir_all(&target_dir)?; + let source_name = command_source_name(source_commands, &source_file); + let description = command_skill_description(&document, &source_name); + fs::write( + target_dir.join("SKILL.md"), + render_command_skill(&document.body, &name, &description, &source_name), + )?; + imported += 1; + } + + Ok(imported) +} + +fn read_external_mcp_servers( + source_root: &Path, + external_agent_home: Option<&Path>, +) -> io::Result> { + let mut servers = BTreeMap::new(); + let project_config_file = external_agent_project_config_file(); + for relative_path in [ + EXTERNAL_AGENT_MCP_CONFIG_FILE.to_string(), + project_config_file.clone(), + ] { + let source_file = source_root.join(&relative_path); + if !source_file.is_file() { + continue; + } + let raw = fs::read_to_string(&source_file)?; + let parsed: JsonValue = serde_json::from_str(&raw) + .map_err(|err| invalid_data_error(format!("invalid MCP config: {err}")))?; + append_mcp_servers_from_value(&parsed, &mut servers, McpServerMerge::Overwrite); + if relative_path == project_config_file + && let Some(projects) = parsed.get("projects").and_then(JsonValue::as_object) + { + for (project_path, project_config) in projects { + if project_path_matches_source_root(project_path, source_root) { + append_mcp_servers_from_value( + project_config, + &mut servers, + McpServerMerge::Overwrite, + ); + } + } + } + } + if let Some(external_agent_root) = external_agent_home.and_then(Path::parent) + && external_agent_root != source_root + { + append_external_agent_project_mcp_servers( + &external_agent_root.join(external_agent_project_config_file()), + source_root, + &mut servers, + )?; + } + + Ok(servers) +} + +fn append_external_agent_project_mcp_servers( + source_file: &Path, + source_root: &Path, + servers: &mut BTreeMap, +) -> io::Result<()> { + if !source_file.is_file() { + return Ok(()); + } + let raw = fs::read_to_string(source_file)?; + let parsed: JsonValue = serde_json::from_str(&raw) + .map_err(|err| invalid_data_error(format!("invalid MCP config: {err}")))?; + let Some(projects) = parsed.get("projects").and_then(JsonValue::as_object) else { + return Ok(()); + }; + for (project_path, project_config) in projects { + if project_path_matches_source_root(project_path, source_root) { + append_mcp_servers_from_value( + project_config, + servers, + McpServerMerge::PreserveExisting, + ); + } + } + Ok(()) +} + +#[derive(Clone, Copy)] +enum McpServerMerge { + Overwrite, + PreserveExisting, +} + +fn append_mcp_servers_from_value( + value: &JsonValue, + servers: &mut BTreeMap, + merge: McpServerMerge, +) { + let Some(mcp_servers) = value.get("mcpServers").and_then(JsonValue::as_object) else { + return; + }; + for (server_name, server_config) in mcp_servers { + match merge { + McpServerMerge::Overwrite => { + servers.insert(server_name.clone(), server_config.clone()); + } + McpServerMerge::PreserveExisting => { + servers + .entry(server_name.clone()) + .or_insert_with(|| server_config.clone()); + } + } + } +} + +fn project_path_matches_source_root(project_path: &str, source_root: &Path) -> bool { + let project_path = Path::new(project_path); + if project_path == source_root { + return true; + } + let Ok(project_path) = project_path.canonicalize() else { + return false; + }; + source_root + .canonicalize() + .is_ok_and(|source_root| source_root == project_path) +} + +fn mcp_server_toml_table( + server_name: &str, + server_config: Option<&serde_json::Map>, + enabled_servers: &[String], + disabled_servers: &BTreeSet, +) -> Option> { + let mut table = toml::map::Map::new(); + let server_config = server_config?; + let transport_type = server_config.get("type").and_then(JsonValue::as_str); + if mcp_server_is_disabled( + server_name, + server_config, + enabled_servers, + disabled_servers, + ) { + return None; + } + + if let Some(command) = server_config.get("command").and_then(json_string) { + if !matches!(transport_type, None | Some("stdio")) { + return None; + } + if contains_env_placeholder(&command) { + return None; + } + table.insert("command".to_string(), TomlValue::String(command)); + if let Some(args) = server_config.get("args") { + let args = json_string_vec(args); + if args.iter().any(|arg| contains_env_placeholder(arg)) { + return None; + } + let args = args.into_iter().map(TomlValue::String).collect::>(); + if !args.is_empty() { + table.insert("args".to_string(), TomlValue::Array(args)); + } + } + if let Some(env) = server_config.get("env").and_then(JsonValue::as_object) { + append_env_config(&mut table, env)?; + } + } else if let Some(url) = server_config.get("url").and_then(json_string) { + if !matches!( + transport_type, + None | Some("http") | Some("streamable_http") + ) { + return None; + } + if contains_env_placeholder(&url) { + return None; + } + table.insert("url".to_string(), TomlValue::String(url)); + if let Some(headers) = server_config.get("headers").and_then(JsonValue::as_object) { + append_header_config(&mut table, headers)?; + } + } else { + return None; + } + + Some(table) +} + +fn mcp_server_is_disabled( + server_name: &str, + server_config: &serde_json::Map, + enabled_servers: &[String], + disabled_servers: &BTreeSet, +) -> bool { + server_config + .get("enabled") + .and_then(JsonValue::as_bool) + .is_some_and(|enabled| !enabled) + || server_config + .get("disabled") + .and_then(JsonValue::as_bool) + .unwrap_or(false) + || (!enabled_servers.is_empty() && !enabled_servers.iter().any(|name| name == server_name)) + || disabled_servers.contains(server_name) +} + +fn append_header_config( + table: &mut toml::map::Map, + headers: &serde_json::Map, +) -> Option<()> { + let mut static_headers = toml::map::Map::new(); + let mut env_headers = toml::map::Map::new(); + + for (key, value) in headers { + let header_value = json_string(value).unwrap_or_else(|| value.to_string()); + if key.eq_ignore_ascii_case("authorization") + && let Some(token_env) = header_value + .strip_prefix("Bearer ") + .and_then(parse_env_placeholder) + { + table.insert( + "bearer_token_env_var".to_string(), + TomlValue::String(token_env), + ); + continue; + } + + if let Some(env_var) = parse_env_placeholder(&header_value) { + env_headers.insert(key.clone(), TomlValue::String(env_var)); + } else if contains_env_placeholder(&header_value) { + return None; + } else { + static_headers.insert(key.clone(), TomlValue::String(header_value)); + } + } + + if !static_headers.is_empty() { + table.insert("http_headers".to_string(), TomlValue::Table(static_headers)); + } + if !env_headers.is_empty() { + table.insert( + "env_http_headers".to_string(), + TomlValue::Table(env_headers), + ); + } + Some(()) +} + +fn append_env_config( + table: &mut toml::map::Map, + env: &serde_json::Map, +) -> Option<()> { + let mut static_env = toml::map::Map::new(); + let mut env_vars = Vec::new(); + + for (key, value) in env { + let env_value = json_string(value).unwrap_or_else(|| value.to_string()); + if parse_env_placeholder(&env_value).as_deref() == Some(key.as_str()) { + env_vars.push(TomlValue::String(key.clone())); + } else if contains_env_placeholder(&env_value) { + return None; + } else { + static_env.insert(key.clone(), TomlValue::String(env_value)); + } + } + + if !env_vars.is_empty() { + table.insert("env_vars".to_string(), TomlValue::Array(env_vars)); + } + if !static_env.is_empty() { + table.insert("env".to_string(), TomlValue::Table(static_env)); + } + Some(()) +} + +fn parse_env_placeholder(value: &str) -> Option { + let inner = value.strip_prefix("${")?.strip_suffix('}')?; + let name = inner + .split_once(":-") + .map_or(inner, |(name, _default)| name); + let mut chars = name.chars(); + let first = chars.next()?; + if !(first == '_' || first.is_ascii_alphabetic()) { + return None; + } + if !chars.all(|ch| ch == '_' || ch.is_ascii_alphanumeric()) { + return None; + } + Some(name.to_string()) +} + +fn contains_env_placeholder(value: &str) -> bool { + value.contains("${") +} + +fn hook_migration( + source_external_agent_dir: &Path, + target_config_dir: Option<&Path>, +) -> io::Result> { + let mut settings_files = Vec::new(); + let mut disable_all_hooks = None; + for settings_name in ["settings.json", "settings.local.json"] { + let settings_file = source_external_agent_dir.join(settings_name); + if !settings_file.is_file() { + continue; + } + let raw = fs::read_to_string(&settings_file)?; + let settings: JsonValue = serde_json::from_str(&raw) + .map_err(|err| invalid_data_error(format!("invalid hooks settings: {err}")))?; + if let Some(disabled) = settings.get("disableAllHooks").and_then(JsonValue::as_bool) { + disable_all_hooks = Some(disabled); + } + settings_files.push(settings); + } + + if disable_all_hooks.unwrap_or(false) { + return Ok(serde_json::Map::new()); + } + + let mut migration = serde_json::Map::new(); + for settings in settings_files { + append_convertible_hook_groups(&settings, &mut migration, target_config_dir); + } + + Ok(migration) +} + +fn append_convertible_hook_groups( + settings: &JsonValue, + hooks_payload: &mut serde_json::Map, + target_config_dir: Option<&Path>, +) { + let Some(hooks_config) = settings.get("hooks").and_then(JsonValue::as_object) else { + return; + }; + + for event_name in HOOK_EVENT_NAMES { + let Some(groups) = hooks_config.get(event_name).and_then(JsonValue::as_array) else { + continue; + }; + for group in groups { + let Some(group_object) = group.as_object() else { + continue; + }; + if group_object.contains_key("if") + || group_object + .keys() + .any(|key| !matches!(key.as_str(), "matcher" | "hooks")) + { + continue; + } + let mut hook_commands = Vec::new(); + if let Some(hooks) = group_object.get("hooks").and_then(JsonValue::as_array) { + for hook in hooks { + let Some(hook_object) = hook.as_object() else { + continue; + }; + let hook_type = hook_object + .get("type") + .and_then(JsonValue::as_str) + .unwrap_or("command"); + if hook_type != "command" { + continue; + } + if hook_object.keys().any(|key| { + !matches!( + key.as_str(), + "type" + | "command" + | "timeout" + | "timeoutSec" + | "statusMessage" + | "async" + ) + }) { + continue; + } + if hook_object + .get("async") + .and_then(JsonValue::as_bool) + .unwrap_or(false) + { + continue; + } + if ["asyncRewake", "shell", "once"] + .into_iter() + .any(|field| hook_object.contains_key(field)) + { + continue; + } + let Some(command) = hook_object + .get("command") + .and_then(JsonValue::as_str) + .map(str::trim) + .filter(|command| !command.is_empty()) + else { + continue; + }; + + let mut command_payload = serde_json::Map::new(); + command_payload + .insert("type".to_string(), JsonValue::String("command".to_string())); + command_payload.insert( + "command".to_string(), + JsonValue::String(rewrite_hook_command(command, target_config_dir)), + ); + if let Some(timeout) = hook_object + .get("timeout") + .or_else(|| hook_object.get("timeoutSec")) + .and_then(json_u64) + { + command_payload.insert( + "timeout".to_string(), + JsonValue::Number(serde_json::Number::from(timeout)), + ); + } + if let Some(status_message) = + hook_object.get("statusMessage").and_then(JsonValue::as_str) + { + command_payload.insert( + "statusMessage".to_string(), + JsonValue::String(rewrite_external_agent_terms(status_message)), + ); + } + hook_commands.push(JsonValue::Object(command_payload)); + } + } + if hook_commands.is_empty() { + continue; + } + + let mut group_payload = serde_json::Map::new(); + if HOOK_EVENT_NAMES_WITH_MATCHERS.contains(&event_name) + && let Some(matcher) = group_object.get("matcher").and_then(JsonValue::as_str) + { + group_payload.insert( + "matcher".to_string(), + JsonValue::String(matcher.to_string()), + ); + } + group_payload.insert("hooks".to_string(), JsonValue::Array(hook_commands)); + if let Some(groups) = hooks_payload + .entry(event_name.to_string()) + .or_insert_with(|| JsonValue::Array(Vec::new())) + .as_array_mut() + { + groups.push(JsonValue::Object(group_payload)); + } + } + } +} + +fn rewrite_hook_command(command: &str, target_config_dir: Option<&Path>) -> String { + let Some(target_config_dir) = target_config_dir else { + return command.to_string(); + }; + let target_hooks_dir = target_config_dir.join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR); + let hook_dir = shell_quote_path_prefix(&target_hooks_dir); + let project_dir_env_var = external_agent_project_dir_env_var(); + let source_hooks_path = format!( + "{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/", + external_agent_config_dir() + ); + let source_hook_prefixes = [ + format!("${{{project_dir_env_var}}}/{source_hooks_path}"), + format!("${project_dir_env_var}/{source_hooks_path}"), + format!("./{source_hooks_path}"), + source_hooks_path.clone(), + ]; + let command = replace_quoted_hook_path_prefixes( + command, + '\'', + &source_hook_prefixes, + &hook_dir, + &target_hooks_dir, + ); + let command = replace_quoted_hook_path_prefixes( + &command, + '"', + &source_hook_prefixes, + &hook_dir, + &target_hooks_dir, + ); + command + .replace( + &format!("\"${project_dir_env_var}\"/{source_hooks_path}"), + &hook_dir, + ) + .replace( + &format!("\"${{{project_dir_env_var}}}\"/{source_hooks_path}"), + &hook_dir, + ) + .replace( + &format!("${{{project_dir_env_var}}}/{source_hooks_path}"), + &hook_dir, + ) + .replace( + &format!("${project_dir_env_var}/{source_hooks_path}"), + &hook_dir, + ) + .replace(&format!("./{source_hooks_path}"), &hook_dir) + .replace(&source_hooks_path, &hook_dir) +} + +fn replace_quoted_hook_path_prefixes( + command: &str, + quote: char, + source_hook_prefixes: &[String], + hook_dir: &str, + target_hooks_dir: &Path, +) -> String { + let mut rewritten = command.to_string(); + for source_hook_prefix in source_hook_prefixes { + let quoted_prefix = format!("{quote}{source_hook_prefix}"); + let mut search_start = 0usize; + while let Some(relative_start) = rewritten[search_start..].find("ed_prefix) { + let start = search_start + relative_start; + let path_start = start + quoted_prefix.len(); + if let Some(relative_end) = rewritten[path_start..].find(quote) { + let end = path_start + relative_end; + let suffix = rewritten[path_start..end].to_string(); + let replacement = + shell_single_quote(target_hooks_dir.join(suffix).to_string_lossy().as_ref()); + rewritten.replace_range(start..end + quote.len_utf8(), &replacement); + search_start = start + replacement.len(); + } else { + rewritten.replace_range(start..path_start, hook_dir); + search_start = start + hook_dir.len(); + } + } + } + rewritten +} + +fn shell_quote_path_prefix(path: &Path) -> String { + format!("{}/", shell_single_quote(path.to_string_lossy().as_ref())) +} + +fn shell_single_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', "'\\''")) +} + +fn copy_hook_scripts(source_external_agent_dir: &Path, target_config_dir: &Path) -> io::Result<()> { + let source_hooks = source_external_agent_dir.join(EXTERNAL_AGENT_HOOKS_SUBDIR); + if !source_hooks.is_dir() { + return Ok(()); + } + let target_hooks = target_config_dir.join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR); + copy_dir_recursive_skip_existing(&source_hooks, &target_hooks) +} + +fn copy_dir_recursive_skip_existing(source: &Path, target: &Path) -> io::Result<()> { + fs::create_dir_all(target)?; + for entry in fs::read_dir(source)? { + let entry = entry?; + let source_path = entry.path(); + let target_path = target.join(entry.file_name()); + let file_type = entry.file_type()?; + if file_type.is_dir() { + copy_dir_recursive_skip_existing(&source_path, &target_path)?; + } else if file_type.is_file() && !target_path.exists() { + fs::copy(source_path, target_path)?; + } + } + Ok(()) +} + +fn agent_source_files(source_agents: &Path) -> io::Result> { + if !source_agents.is_dir() { + return Ok(Vec::new()); + } + + let mut files = Vec::new(); + for entry in fs::read_dir(source_agents)? { + let entry = entry?; + let path = entry.path(); + if !entry.file_type()?.is_file() + || path.extension().and_then(|ext| ext.to_str()) != Some("md") + { + continue; + } + if path.file_stem().and_then(|stem| stem.to_str()) == Some("README") { + continue; + } + files.push(path); + } + files.sort(); + Ok(files) +} + +fn subagent_target_file(source_file: &Path, target_agents: &Path) -> Option { + Some(target_agents.join(format!("{}.toml", source_file.file_stem()?.to_str()?))) +} + +fn command_source_files(source_commands: &Path) -> io::Result> { + let mut files = Vec::new(); + collect_markdown_files(source_commands, &mut files)?; + files.sort(); + Ok(files) +} + +fn unique_supported_command_sources(source_commands: &Path) -> io::Result> { + let mut by_name = BTreeMap::>::new(); + for source_file in command_source_files(source_commands)? { + let document = parse_document(&source_file)?; + let Some(name) = command_skill_name_if_supported(source_commands, &source_file, &document) + else { + continue; + }; + by_name.entry(name).or_default().push(source_file); + } + + Ok(by_name + .into_iter() + .filter_map(|(name, source_files)| { + let [source_file] = source_files.as_slice() else { + return None; + }; + Some((source_file.clone(), name)) + }) + .collect()) +} + +fn collect_markdown_files(dir: &Path, files: &mut Vec) -> io::Result<()> { + if !dir.is_dir() { + return Ok(()); + } + + for entry in fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + let file_type = entry.file_type()?; + if file_type.is_dir() { + collect_markdown_files(&path, files)?; + } else if file_type.is_file() && path.extension().and_then(|ext| ext.to_str()) == Some("md") + { + files.push(path); + } + } + Ok(()) +} + +fn parse_document(source_file: &Path) -> io::Result { + let content = fs::read_to_string(source_file)?; + Ok(parse_document_content(&content)) +} + +fn parse_document_content(content: &str) -> ParsedDocument { + let Some(rest) = content + .strip_prefix("---\n") + .or_else(|| content.strip_prefix("---\r\n")) + else { + return ParsedDocument { + frontmatter: BTreeMap::new(), + body: content.to_string(), + frontmatter_error: None, + }; + }; + let Some((end, body_start)) = frontmatter_end(rest) else { + return ParsedDocument { + frontmatter: BTreeMap::new(), + body: content.to_string(), + frontmatter_error: None, + }; + }; + + let raw_frontmatter = &rest[..end]; + let body = &rest[body_start..]; + let (frontmatter, frontmatter_error) = parse_frontmatter(raw_frontmatter); + ParsedDocument { + frontmatter, + body: body.to_string(), + frontmatter_error, + } +} + +fn frontmatter_end(rest: &str) -> Option<(usize, usize)> { + [ + "\r\n---\r\n", + "\r\n---\n", + "\n---\r\n", + "\n---\n", + "\r\n---", + "\n---", + ] + .into_iter() + .filter_map(|delimiter| rest.find(delimiter).map(|end| (end, end + delimiter.len()))) + .min_by_key(|(end, _body_start)| *end) +} + +fn parse_frontmatter( + raw_frontmatter: &str, +) -> (BTreeMap, Option) { + let parsed: YamlValue = match serde_yaml::from_str(raw_frontmatter) { + Ok(parsed) => parsed, + Err(err) => return (BTreeMap::new(), Some(err.to_string())), + }; + let Some(mapping) = parsed.as_mapping() else { + return ( + BTreeMap::new(), + Some("frontmatter is not a YAML mapping".to_string()), + ); + }; + + let mut frontmatter = BTreeMap::new(); + for (key, value) in mapping { + let Some(key) = key.as_str().map(str::trim).filter(|key| !key.is_empty()) else { + continue; + }; + frontmatter.insert(key.to_string(), frontmatter_value_from_yaml(value)); + } + + (frontmatter, None) +} + +fn frontmatter_value_from_yaml(value: &YamlValue) -> FrontmatterValue { + match value { + YamlValue::String(value) => FrontmatterValue::Scalar(value.trim().to_string()), + YamlValue::Bool(value) => FrontmatterValue::Scalar(value.to_string()), + YamlValue::Number(value) => FrontmatterValue::Scalar(value.to_string()), + YamlValue::Null | YamlValue::Sequence(_) | YamlValue::Mapping(_) | YamlValue::Tagged(_) => { + FrontmatterValue::Other + } + } +} + +fn agent_metadata(document: &ParsedDocument) -> Option { + if document.frontmatter_error.is_some() || document.body.trim().is_empty() { + return None; + } + let name = document + .frontmatter + .get("name") + .and_then(FrontmatterValue::as_scalar) + .filter(|value| !value.trim().is_empty()) + .map(ToOwned::to_owned)?; + + let description = document + .frontmatter + .get("description") + .and_then(FrontmatterValue::as_scalar) + .filter(|value| !value.trim().is_empty()) + .map(ToOwned::to_owned)?; + + Some(AgentMetadata { + name, + description, + permission_mode: frontmatter_string(&document.frontmatter, "permissionMode"), + effort: frontmatter_string(&document.frontmatter, "effort"), + }) +} + +fn render_agent_toml(body: &str, metadata: &AgentMetadata) -> io::Result { + let mut document = toml::map::Map::new(); + document.insert("name".to_string(), TomlValue::String(metadata.name.clone())); + document.insert( + "description".to_string(), + TomlValue::String(rewrite_external_agent_terms(&metadata.description)), + ); + if let Some(effort) = metadata.effort.as_ref() + && let Some(effort) = map_agent_reasoning_effort(effort) + { + document.insert( + "model_reasoning_effort".to_string(), + TomlValue::String(effort), + ); + } + if let Some(sandbox_mode) = metadata + .permission_mode + .as_deref() + .and_then(map_agent_permission_mode) + { + document.insert( + "sandbox_mode".to_string(), + TomlValue::String(sandbox_mode.to_string()), + ); + } + document.insert( + "developer_instructions".to_string(), + TomlValue::String(render_agent_body(body)), + ); + + let serialized = toml::to_string_pretty(&TomlValue::Table(document)) + .map_err(|err| invalid_data_error(format!("failed to serialize agent TOML: {err}")))?; + Ok(format!("{}\n", serialized.trim_end())) +} + +fn render_agent_body(body: &str) -> String { + let body = rewrite_external_agent_terms(body.trim()); + if body.is_empty() { + "No subagent instructions were found.".to_string() + } else { + body + } +} + +fn command_skill_name(source_commands: &Path, source_file: &Path) -> String { + slugify_name(&format!( + "{COMMAND_SKILL_PREFIX}-{}", + command_source_name(source_commands, source_file) + )) +} + +fn command_skill_name_if_supported( + source_commands: &Path, + source_file: &Path, + document: &ParsedDocument, +) -> Option { + let name = command_skill_name(source_commands, source_file); + if name.chars().count() > MAX_SKILL_NAME_LEN { + return None; + } + let source_name = command_source_name(source_commands, source_file); + let description = command_skill_description(document, &source_name); + if description.chars().count() > MAX_SKILL_DESCRIPTION_LEN { + return None; + } + if has_unsupported_command_template_features(&document.body) { + return None; + } + Some(name) +} + +fn command_skill_description(document: &ParsedDocument, source_name: &str) -> String { + document + .frontmatter + .get("description") + .and_then(FrontmatterValue::as_scalar) + .filter(|value| !value.trim().is_empty()) + .map(ToOwned::to_owned) + .unwrap_or_else(|| format!("Run the migrated source command `{source_name}`.")) +} + +fn command_source_name(source_commands: &Path, source_file: &Path) -> String { + source_file + .strip_prefix(source_commands) + .unwrap_or(source_file) + .with_extension("") + .components() + .filter_map(|component| component.as_os_str().to_str()) + .collect::>() + .join("-") +} + +fn render_command_skill(body: &str, name: &str, description: &str, source_name: &str) -> String { + let body = rewrite_external_agent_terms(body.trim()); + let template_body = if body.is_empty() { + "No command template body was found.".to_string() + } else { + body + }; + format!( + "---\nname: {}\ndescription: {}\n---\n\n# {name}\n\nUse this skill when the user asks to run the migrated source command `{source_name}`.\n\n## Command Template\n\n{template_body}\n", + yaml_string(name), + yaml_string(&rewrite_external_agent_terms(description)), + ) +} + +fn has_unsupported_command_template_features(template: &str) -> bool { + template.contains("$ARGUMENTS") + || contains_numbered_argument_placeholder(template) + || (template.contains("{{") && template.contains("}}")) + || template.contains("!`") + || template.contains("! `") + || template + .split_whitespace() + .any(|token| token.strip_prefix('@').is_some_and(|rest| !rest.is_empty())) +} + +fn contains_numbered_argument_placeholder(template: &str) -> bool { + let bytes = template.as_bytes(); + bytes + .windows(2) + .any(|window| window[0] == b'$' && window[1].is_ascii_digit()) +} + +fn frontmatter_string( + frontmatter: &BTreeMap, + key: &str, +) -> Option { + frontmatter + .get(key) + .and_then(FrontmatterValue::as_scalar) + .map(ToOwned::to_owned) +} + +fn map_agent_reasoning_effort(effort: &str) -> Option { + let mapped = match effort { + "max" => "xhigh".to_string(), + _ => effort.to_string(), + }; + matches!( + mapped.as_str(), + "none" | "minimal" | "low" | "medium" | "high" | "xhigh" + ) + .then_some(mapped) +} + +fn map_agent_permission_mode(permission_mode: &str) -> Option<&'static str> { + match permission_mode { + "acceptEdits" => Some("workspace-write"), + "readOnly" => Some("read-only"), + _ => None, + } +} + +fn json_string_vec(value: &JsonValue) -> Vec { + match value { + JsonValue::Array(values) => values.iter().filter_map(json_string).collect(), + _ => json_string(value).into_iter().collect(), + } +} + +fn json_string(value: &JsonValue) -> Option { + match value { + JsonValue::Null => None, + JsonValue::String(value) => Some(value.clone()), + JsonValue::Bool(value) => Some(value.to_string()), + JsonValue::Number(value) => Some(value.to_string()), + JsonValue::Array(_) | JsonValue::Object(_) => None, + } +} + +fn json_u64(value: &JsonValue) -> Option { + if value.is_boolean() || value.is_null() { + return None; + } + value.as_u64().or_else(|| value.as_str()?.parse().ok()) +} + +fn yaml_string(value: &str) -> String { + format!("\"{}\"", value.replace('\\', "\\\\").replace('"', "\\\"")) +} + +fn slugify_name(value: &str) -> String { + let mut slug = String::new(); + let mut last_was_dash = false; + for ch in value.chars() { + if ch.is_ascii_alphanumeric() { + slug.push(ch.to_ascii_lowercase()); + last_was_dash = false; + } else if !last_was_dash { + slug.push('-'); + last_was_dash = true; + } + } + + let slug = slug.trim_matches('-').to_string(); + if slug.is_empty() { + "migrated".to_string() + } else { + slug + } +} + +impl FrontmatterValue { + fn as_scalar(&self) -> Option<&str> { + match self { + Self::Scalar(value) => Some(value), + Self::Other => None, + } + } +} + +fn is_missing_or_empty_text_file(path: &Path) -> io::Result { + if !path.exists() { + return Ok(true); + } + if !path.is_file() { + return Ok(false); + } + + Ok(fs::read_to_string(path)?.trim().is_empty()) +} + +fn rewrite_external_agent_terms(content: &str) -> String { + let mut rewritten = replace_case_insensitive_with_boundaries( + content, + &external_agent_doc_file_name(), + "AGENTS.md", + ); + for from in external_agent_term_variants() { + rewritten = replace_case_insensitive_with_boundaries(&rewritten, &from, "Codex"); + } + rewritten +} + +fn replace_case_insensitive_with_boundaries( + input: &str, + needle: &str, + replacement: &str, +) -> String { + let needle_lower = needle.to_ascii_lowercase(); + if needle_lower.is_empty() { + return input.to_string(); + } + + let haystack_lower = input.to_ascii_lowercase(); + let bytes = input.as_bytes(); + let mut output = String::with_capacity(input.len()); + let mut last_emitted = 0usize; + let mut search_start = 0usize; + + while let Some(relative_pos) = haystack_lower[search_start..].find(&needle_lower) { + let start = search_start + relative_pos; + let end = start + needle_lower.len(); + let boundary_before = start == 0 || !is_word_byte(bytes[start - 1]); + let boundary_after = end == bytes.len() || !is_word_byte(bytes[end]); + + if boundary_before && boundary_after { + output.push_str(&input[last_emitted..start]); + output.push_str(replacement); + last_emitted = end; + } + + search_start = start + 1; + } + + if last_emitted == 0 { + return input.to_string(); + } + + output.push_str(&input[last_emitted..]); + output +} + +fn is_word_byte(byte: u8) -> bool { + byte.is_ascii_alphanumeric() || byte == b'_' +} + +fn invalid_data_error(message: impl Into) -> io::Error { + io::Error::new(io::ErrorKind::InvalidData, message.into()) +} + +fn external_agent_config_dir() -> String { + format!(".{SOURCE_EXTERNAL_AGENT_NAME}") +} + +fn external_agent_project_config_file() -> String { + format!(".{SOURCE_EXTERNAL_AGENT_NAME}.json") +} + +fn external_agent_project_dir_env_var() -> String { + format!( + "{}_PROJECT_DIR", + SOURCE_EXTERNAL_AGENT_NAME.to_ascii_uppercase() + ) +} + +fn external_agent_doc_file_name() -> String { + format!("{SOURCE_EXTERNAL_AGENT_NAME}.md") +} + +fn external_agent_term_variants() -> [String; 5] { + [ + format!("{SOURCE_EXTERNAL_AGENT_NAME} code"), + format!("{SOURCE_EXTERNAL_AGENT_NAME}-code"), + format!("{SOURCE_EXTERNAL_AGENT_NAME}_code"), + format!("{SOURCE_EXTERNAL_AGENT_NAME}code"), + SOURCE_EXTERNAL_AGENT_NAME.to_string(), + ] +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + fn source_path(relative_path: &str) -> PathBuf { + Path::new("/repo") + .join(external_agent_config_dir()) + .join(relative_path) + } + + fn source_hook_command(script_name: &str) -> String { + format!( + "python3 {}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/{script_name}", + external_agent_config_dir() + ) + } + + fn source_hook_command_with_project_dir(script_name: &str) -> String { + format!( + "python3 \"${}\"/{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/{script_name}", + external_agent_project_dir_env_var(), + external_agent_config_dir() + ) + } + + fn migrated_hook_command(script_name: &str) -> String { + let hook_dir = shell_quote_path_prefix( + &Path::new("/repo/.codex").join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR), + ); + format!("python3 {hook_dir}{script_name}") + } + + fn migrated_quoted_hook_command(script_name: &str) -> String { + let hook_path = Path::new("/repo/.codex") + .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) + .join(script_name); + format!( + "python3 {}", + shell_single_quote(hook_path.to_string_lossy().as_ref()) + ) + } + + #[test] + fn env_placeholder_accepts_defaults() { + assert_eq!( + parse_env_placeholder("${TOKEN:-fallback}"), + Some("TOKEN".to_string()) + ); + } + + #[test] + fn mcp_migration_skips_placeholder_args() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join(".mcp.json"), + r#"{"mcpServers":{"db":{"command":"db-server","args":["${DATABASE_URL}"]}}}"#, + ) + .expect("write mcp"); + + assert_eq!( + build_mcp_config_from_external( + root.path(), + /*external_agent_home*/ None, + /*settings*/ None, + ) + .unwrap(), + TomlValue::Table(Default::default()) + ); + } + + #[test] + fn mcp_migration_skips_unsupported_transports() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join(".mcp.json"), + r#"{ + "mcpServers": { + "legacy-sse": {"type": "sse", "url": "https://example.invalid/sse"}, + "vault": { + "url": "https://example.invalid/vault", + "headers": {"Authorization": "Bearer ${VAULT_TOKEN:-dev-token}"} + } + } + }"#, + ) + .expect("write mcp"); + + assert_eq!( + build_mcp_config_from_external( + root.path(), + /*external_agent_home*/ None, + /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.vault] +url = "https://example.invalid/vault" +bearer_token_env_var = "VAULT_TOKEN" +"# + ) + .unwrap() + ); + } + + #[test] + fn mcp_migration_reads_matching_project_entries_from_repo_external_project_config() { + let root = tempfile::TempDir::new().expect("tempdir"); + let project = root.path().join("repo"); + fs::create_dir_all(&project).expect("create repo"); + let other = root.path().join("other"); + fs::create_dir_all(&other).expect("create other"); + fs::write( + project.join(external_agent_project_config_file()), + serde_json::json!({ + "mcpServers": { + "top": {"command": "top-server"} + }, + "projects": { + project.display().to_string(): { + "mcpServers": { + "repo": {"command": "repo-server"} + } + }, + other.display().to_string(): { + "mcpServers": { + "other": {"command": "other-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + assert_eq!( + build_mcp_config_from_external( + &project, /*external_agent_home*/ None, /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.repo] +command = "repo-server" + +[mcp_servers.top] +command = "top-server" +"# + ) + .unwrap() + ); + } + + #[test] + fn mcp_migration_reads_matching_project_entries_from_home_external_project_config() { + let root = tempfile::TempDir::new().expect("tempdir"); + let project = root.path().join("repo"); + fs::create_dir_all(&project).expect("create repo"); + let external_agent_home = root.path().join(external_agent_config_dir()); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + root.path().join(external_agent_project_config_file()), + serde_json::json!({ + "projects": { + project.display().to_string(): { + "mcpServers": { + "repo": {"command": "repo-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + assert_eq!( + build_mcp_config_from_external( + &project, + Some(&external_agent_home), + /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.repo] +command = "repo-server" +"# + ) + .unwrap() + ); + } + + #[test] + fn mcp_migration_preserves_repo_servers_over_home_project_entries() { + let root = tempfile::TempDir::new().expect("tempdir"); + let project = root.path().join("repo"); + fs::create_dir_all(&project).expect("create repo"); + let external_agent_home = root.path().join(external_agent_config_dir()); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + project.join(EXTERNAL_AGENT_MCP_CONFIG_FILE), + serde_json::json!({ + "mcpServers": { + "shared": {"command": "repo-server"} + } + }) + .to_string(), + ) + .expect("write repo mcp"); + fs::write( + root.path().join(external_agent_project_config_file()), + serde_json::json!({ + "projects": { + project.display().to_string(): { + "mcpServers": { + "home-only": {"command": "home-only-server"}, + "shared": {"command": "home-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + assert_eq!( + build_mcp_config_from_external( + &project, + Some(&external_agent_home), + /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.home-only] +command = "home-only-server" + +[mcp_servers.shared] +command = "repo-server" +"# + ) + .unwrap() + ); + } + + #[test] + fn mcp_migration_skips_disabled_servers() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join(".mcp.json"), + r#"{ + "mcpServers": { + "enabled": {"command": "enabled-server"}, + "explicit-disabled": {"command": "disabled-server", "disabled": true}, + "not-enabled": {"command": "not-enabled-server"} + } + }"#, + ) + .expect("write mcp"); + let settings = serde_json::json!({ + "enabledMcpjsonServers": ["enabled"], + "disabledMcpjsonServers": ["explicit-disabled"] + }); + + assert_eq!( + build_mcp_config_from_external( + root.path(), + /*external_agent_home*/ None, + Some(&settings), + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.enabled] +command = "enabled-server" +"# + ) + .unwrap() + ); + } + + #[test] + fn command_skill_names_include_nested_paths() { + let root = source_path("commands"); + let file = source_path("commands/pr/review.md"); + + assert_eq!(command_skill_name(&root, &file), "source-command-pr-review"); + } + + #[test] + fn command_skill_names_must_fit_codex_skill_loader_limit() { + let root = source_path("commands"); + let file = source_path("commands/this/is/a/deeply/nested/command/with/a/very/long/name.md"); + let document = parse_document_content("---\ndescription: Review PR\n---\nReview\n"); + + assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); + } + + #[test] + fn commands_with_provider_runtime_expansion_are_skipped() { + let root = source_path("commands"); + let file = source_path("commands/deploy.md"); + let document = parse_document_content( + "---\ndescription: Deploy\n---\nDeploy $ARGUMENTS from @release.yaml\n", + ); + + assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); + } + + #[test] + fn command_slug_collisions_are_skipped() { + let root = tempfile::TempDir::new().expect("tempdir"); + let commands = root.path().join("commands"); + fs::create_dir_all(&commands).expect("create commands"); + fs::write( + commands.join("foo-bar.md"), + "---\ndescription: First\n---\nRun the first command.\n", + ) + .expect("write first command"); + fs::write( + commands.join("foo_bar.md"), + "---\ndescription: Second\n---\nRun the second command.\n", + ) + .expect("write second command"); + + assert_eq!( + unique_supported_command_sources(&commands).unwrap(), + Vec::<(PathBuf, String)>::new() + ); + } + + #[test] + fn subagent_accepts_yaml_block_lists_by_ignoring_unsupported_fields() { + let document = parse_document_content( + "---\nname: cloud-incident\ndescription: Debug incidents\nskills:\n - runbook-reader\ntools:\n - Read\n - Bash\ndisallowedTools:\n - Write\n---\nInvestigate carefully.\n", + ); + + assert!(agent_metadata(&document).is_some()); + } + + #[test] + fn subagent_requires_minimum_codex_agent_fields() { + let missing_description = + parse_document_content("---\nname: incomplete\n---\nInvestigate carefully.\n"); + let missing_body = + parse_document_content("---\nname: incomplete\ndescription: Missing body\n---\n"); + + assert!(agent_metadata(&missing_description).is_none()); + assert!(agent_metadata(&missing_body).is_none()); + } + + #[test] + fn subagent_preserves_default_model_when_source_model_is_present() { + let document = parse_document_content( + "---\nname: reviewer\ndescription: Review code\nmodel: source-opus\neffort: max\n---\nReview carefully.\n", + ); + let metadata = agent_metadata(&document).expect("metadata"); + let rendered: TomlValue = + toml::from_str(&render_agent_toml(&document.body, &metadata).expect("render agent")) + .expect("parse rendered agent"); + let expected: TomlValue = toml::from_str( + r#" +name = "reviewer" +description = "Review code" +model_reasoning_effort = "xhigh" +developer_instructions = """ +Review carefully.""" +"#, + ) + .expect("parse expected agent"); + + assert_eq!(rendered, expected); + } + + #[test] + fn subagent_target_preserves_dotted_file_stem() { + let target_agents = Path::new("/repo/.codex/agents"); + let source_file = source_path("agents/security.audit.md"); + + assert_eq!( + subagent_target_file(&source_file, target_agents), + Some(PathBuf::from("/repo/.codex/agents/security.audit.toml")) + ); + } + + #[test] + fn frontmatter_accepts_crlf_delimiters() { + let document = parse_document_content( + "---\r\nname: reviewer\r\ndescription: Review code\r\n---\r\nReview carefully.\r\n", + ); + + assert_eq!( + ( + document + .frontmatter + .get("name") + .and_then(FrontmatterValue::as_scalar), + document + .frontmatter + .get("description") + .and_then(FrontmatterValue::as_scalar), + document.body.as_str(), + ), + ( + Some("reviewer"), + Some("Review code"), + "Review carefully.\r\n" + ) + ); + } + + #[test] + fn hook_migration_ignores_unsupported_handlers() { + let settings = serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "if": "tool_input.command contains 'rm'", + "hooks": [{ + "type": "command", + "command": source_hook_command("policy_gate.py") + }] + }, { + "matcher": "Edit", + "hooks": [ + { + "type": "command", + "if": "Bash(rm *)", + "command": source_hook_command("policy_gate.py") + }, + { + "type": "http", + "url": "https://example.invalid/hook" + } + ] + }], + "PermissionRequest": [{ + "matcher": "Bash", + "hooks": [{ + "type": "command", + "command": source_hook_command("approve.py") + }] + }], + "SubagentStart": [{ + "matcher": "worker", + "hooks": [{"type": "prompt", "prompt": "check"}] + }] + } + }); + let mut migration = serde_json::Map::new(); + append_convertible_hook_groups(&settings, &mut migration, Some(Path::new("/repo/.codex"))); + + assert_eq!( + migration, + serde_json::json!({ + "PermissionRequest": [{ + "matcher": "Bash", + "hooks": [{ + "type": "command", + "command": migrated_hook_command("approve.py") + }] + }] + }) + .as_object() + .cloned() + .expect("object") + ); + } + + #[test] + fn hook_migration_honors_disable_all_hooks() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join("settings.json"), + r#"{ + "disableAllHooks": true, + "hooks": { + "SessionStart": [{ + "matcher": "startup", + "hooks": [{"type": "command", "command": "echo setup"}] + }] + } + }"#, + ) + .expect("write settings"); + + assert_eq!( + hook_migration(root.path(), /*target_config_dir*/ None).unwrap(), + serde_json::Map::new() + ); + } + + #[test] + fn hook_migration_honors_settings_local_disable_override() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join("settings.json"), + r#"{ + "disableAllHooks": true, + "hooks": { + "SessionStart": [{ + "matcher": "project", + "hooks": [{"type": "command", "command": "echo project"}] + }] + } + }"#, + ) + .expect("write project settings"); + fs::write( + root.path().join("settings.local.json"), + r#"{ + "disableAllHooks": false, + "hooks": { + "SessionStart": [{ + "matcher": "local", + "hooks": [{"type": "command", "command": "echo local"}] + }] + } + }"#, + ) + .expect("write local settings"); + + assert_eq!( + hook_migration(root.path(), /*target_config_dir*/ None).unwrap(), + serde_json::json!({ + "SessionStart": [{ + "matcher": "project", + "hooks": [{ + "type": "command", + "command": "echo project" + }] + }, { + "matcher": "local", + "hooks": [{ + "type": "command", + "command": "echo local" + }] + }] + }) + .as_object() + .cloned() + .expect("object") + ); + } + + #[test] + fn hook_command_paths_rewrite_to_target_hook_dir() { + let project_dir_env_var = external_agent_project_dir_env_var(); + let source_hooks_path = format!( + "{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}", + external_agent_config_dir() + ); + assert_eq!( + rewrite_hook_command( + &source_hook_command_with_project_dir("check.py"), + Some(Path::new("/repo/.codex")), + ), + migrated_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command( + &source_hook_command("check.py"), + Some(Path::new("/repo/.codex")), + ), + migrated_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 ./{source_hooks_path}/check.py"), + Some(Path::new("/repo/.codex")), + ), + migrated_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 '${{{project_dir_env_var}}}/{source_hooks_path}/check.py'"), + Some(Path::new("/repo/.codex")), + ), + migrated_quoted_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 \"${{{project_dir_env_var}}}/{source_hooks_path}/check.py\""), + Some(Path::new("/repo/.codex")), + ), + migrated_quoted_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command( + &format!("python3 '${{{project_dir_env_var}}}/{source_hooks_path}/my script.py'"), + Some(Path::new("/repo/.codex")), + ), + migrated_quoted_hook_command("my script.py") + ); + } + + #[test] + fn hook_script_copy_keeps_existing_target_scripts() { + let root = tempfile::TempDir::new().expect("tempdir"); + let source_external_agent_dir = root.path().join(external_agent_config_dir()); + let source_hooks = source_external_agent_dir.join(EXTERNAL_AGENT_HOOKS_SUBDIR); + let target_config_dir = root.path().join(".codex"); + let target_hooks = target_config_dir.join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR); + fs::create_dir_all(&source_hooks).expect("create source hooks"); + fs::create_dir_all(&target_hooks).expect("create target hooks"); + fs::write(source_hooks.join("check.py"), "new script").expect("write source hook"); + fs::write(target_hooks.join("check.py"), "existing script").expect("write target hook"); + + copy_hook_scripts(&source_external_agent_dir, &target_config_dir).expect("copy hooks"); + + assert_eq!( + fs::read_to_string(target_hooks.join("check.py")).expect("read target hook"), + "existing script" + ); + } + + #[test] + fn hook_migration_drops_negative_timeouts() { + let settings = serde_json::json!({ + "hooks": { + "SessionStart": [{ + "matcher": "startup", + "hooks": [{ + "type": "command", + "command": "echo setup", + "timeout": -1 + }] + }] + } + }); + let mut migration = serde_json::Map::new(); + append_convertible_hook_groups(&settings, &mut migration, /*target_config_dir*/ None); + + assert_eq!( + migration, + serde_json::json!({ + "SessionStart": [{ + "matcher": "startup", + "hooks": [{ + "type": "command", + "command": "echo setup" + }] + }] + }) + .as_object() + .cloned() + .expect("object") + ); + } +} diff --git a/codex-rs/hooks/src/lib.rs b/codex-rs/hooks/src/lib.rs index c8358c678..b2958e711 100644 --- a/codex-rs/hooks/src/lib.rs +++ b/codex-rs/hooks/src/lib.rs @@ -5,6 +5,28 @@ mod registry; mod schema; mod types; +/// Hook event names as they appear in hooks JSON and config files. +pub const HOOK_EVENT_NAMES: [&str; 6] = [ + "PreToolUse", + "PermissionRequest", + "PostToolUse", + "SessionStart", + "UserPromptSubmit", + "Stop", +]; + +/// Hook event names whose matcher fields are meaningful during dispatch. +/// +/// Other events can appear in hooks JSON, but Codex ignores their matcher +/// fields because those events do not dispatch against a tool or session-start +/// source. +pub const HOOK_EVENT_NAMES_WITH_MATCHERS: [&str; 4] = [ + "PreToolUse", + "PermissionRequest", + "PostToolUse", + "SessionStart", +]; + pub use events::permission_request::PermissionRequestDecision; pub use events::permission_request::PermissionRequestOutcome; pub use events::permission_request::PermissionRequestRequest; diff --git a/codex-rs/tui/src/external_agent_config_migration.rs b/codex-rs/tui/src/external_agent_config_migration.rs index 3046168a8..fa6dcc6e4 100644 --- a/codex-rs/tui/src/external_agent_config_migration.rs +++ b/codex-rs/tui/src/external_agent_config_migration.rs @@ -799,7 +799,7 @@ mod tests { plugin_names: vec!["warehouse".to_string()], }, ], - sessions: Vec::new(), + ..Default::default() } }