From de2b8236cb2873a5ca855b100586f9a0caaafdd2 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 22 May 2026 12:34:00 +0100 Subject: [PATCH 1/5] chore: drop old profile stuff from app server --- .../schema/json/ServerNotification.json | 2 +- .../codex_app_server_protocol.schemas.json | 114 --------------- .../codex_app_server_protocol.v2.schemas.json | 114 --------------- .../schema/json/v2/ConfigReadResponse.json | 114 --------------- .../schema/typescript/v2/Config.ts | 3 +- .../schema/typescript/v2/ProfileV2.ts | 18 --- .../schema/typescript/v2/index.ts | 1 - codex-rs/app-server-protocol/src/lib.rs | 1 - .../app-server-protocol/src/protocol/v1.rs | 14 -- .../src/protocol/v2/config.rs | 42 ++---- .../src/protocol/v2/tests.rs | 138 ------------------ codex-rs/app-server/README.md | 2 +- .../app-server/src/config_manager_service.rs | 22 ++- .../src/config_manager_service_tests.rs | 134 +++++++++++------ .../app-server/tests/suite/v2/config_rpc.rs | 35 ++--- codex-rs/config/src/config_toml.rs | 45 ------ codex-rs/config/src/profile_toml.rs | 14 -- 17 files changed, 136 insertions(+), 677 deletions(-) delete mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/ProfileV2.ts diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index dfb999cf314f..90899cb15278 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -6546,4 +6546,4 @@ } ], "title": "ServerNotification" -} +} \ No newline at end of file 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 155afbb29b0c..576ee86b522a 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 @@ -7361,19 +7361,6 @@ } ] }, - "profile": { - "type": [ - "string", - "null" - ] - }, - "profiles": { - "additionalProperties": { - "$ref": "#/definitions/v2/ProfileV2" - }, - "default": {}, - "type": "object" - }, "review_model": { "type": [ "string", @@ -13173,107 +13160,6 @@ ], "type": "object" }, - "ProfileV2": { - "additionalProperties": true, - "properties": { - "approval_policy": { - "anyOf": [ - { - "$ref": "#/definitions/v2/AskForApproval" - }, - { - "type": "null" - } - ] - }, - "approvals_reviewer": { - "anyOf": [ - { - "$ref": "#/definitions/v2/ApprovalsReviewer" - }, - { - "type": "null" - } - ], - "description": "[UNSTABLE] Optional profile-level override for where approval requests are routed for review. If omitted, the enclosing config default is used." - }, - "chatgpt_base_url": { - "type": [ - "string", - "null" - ] - }, - "model": { - "type": [ - "string", - "null" - ] - }, - "model_provider": { - "type": [ - "string", - "null" - ] - }, - "model_reasoning_effort": { - "anyOf": [ - { - "$ref": "#/definitions/v2/ReasoningEffort" - }, - { - "type": "null" - } - ] - }, - "model_reasoning_summary": { - "anyOf": [ - { - "$ref": "#/definitions/v2/ReasoningSummary" - }, - { - "type": "null" - } - ] - }, - "model_verbosity": { - "anyOf": [ - { - "$ref": "#/definitions/v2/Verbosity" - }, - { - "type": "null" - } - ] - }, - "service_tier": { - "type": [ - "string", - "null" - ] - }, - "tools": { - "anyOf": [ - { - "$ref": "#/definitions/v2/ToolsV2" - }, - { - "type": "null" - } - ] - }, - "web_search": { - "anyOf": [ - { - "$ref": "#/definitions/v2/WebSearchMode" - }, - { - "type": "null" - } - ] - } - }, - "type": "object" - }, "RateLimitReachedType": { "enum": [ "rate_limit_reached", 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 a636cf1de38e..5bb649e6bf9e 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 @@ -3730,19 +3730,6 @@ } ] }, - "profile": { - "type": [ - "string", - "null" - ] - }, - "profiles": { - "additionalProperties": { - "$ref": "#/definitions/ProfileV2" - }, - "default": {}, - "type": "object" - }, "review_model": { "type": [ "string", @@ -9702,107 +9689,6 @@ ], "type": "object" }, - "ProfileV2": { - "additionalProperties": true, - "properties": { - "approval_policy": { - "anyOf": [ - { - "$ref": "#/definitions/AskForApproval" - }, - { - "type": "null" - } - ] - }, - "approvals_reviewer": { - "anyOf": [ - { - "$ref": "#/definitions/ApprovalsReviewer" - }, - { - "type": "null" - } - ], - "description": "[UNSTABLE] Optional profile-level override for where approval requests are routed for review. If omitted, the enclosing config default is used." - }, - "chatgpt_base_url": { - "type": [ - "string", - "null" - ] - }, - "model": { - "type": [ - "string", - "null" - ] - }, - "model_provider": { - "type": [ - "string", - "null" - ] - }, - "model_reasoning_effort": { - "anyOf": [ - { - "$ref": "#/definitions/ReasoningEffort" - }, - { - "type": "null" - } - ] - }, - "model_reasoning_summary": { - "anyOf": [ - { - "$ref": "#/definitions/ReasoningSummary" - }, - { - "type": "null" - } - ] - }, - "model_verbosity": { - "anyOf": [ - { - "$ref": "#/definitions/Verbosity" - }, - { - "type": "null" - } - ] - }, - "service_tier": { - "type": [ - "string", - "null" - ] - }, - "tools": { - "anyOf": [ - { - "$ref": "#/definitions/ToolsV2" - }, - { - "type": "null" - } - ] - }, - "web_search": { - "anyOf": [ - { - "$ref": "#/definitions/WebSearchMode" - }, - { - "type": "null" - } - ] - } - }, - "type": "object" - }, "RateLimitReachedType": { "enum": [ "rate_limit_reached", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json index 7595f7fd0093..4a104b3bd519 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -352,19 +352,6 @@ } ] }, - "profile": { - "type": [ - "string", - "null" - ] - }, - "profiles": { - "additionalProperties": { - "$ref": "#/definitions/ProfileV2" - }, - "default": {}, - "type": "object" - }, "review_model": { "type": [ "string", @@ -642,107 +629,6 @@ ], "type": "string" }, - "ProfileV2": { - "additionalProperties": true, - "properties": { - "approval_policy": { - "anyOf": [ - { - "$ref": "#/definitions/AskForApproval" - }, - { - "type": "null" - } - ] - }, - "approvals_reviewer": { - "anyOf": [ - { - "$ref": "#/definitions/ApprovalsReviewer" - }, - { - "type": "null" - } - ], - "description": "[UNSTABLE] Optional profile-level override for where approval requests are routed for review. If omitted, the enclosing config default is used." - }, - "chatgpt_base_url": { - "type": [ - "string", - "null" - ] - }, - "model": { - "type": [ - "string", - "null" - ] - }, - "model_provider": { - "type": [ - "string", - "null" - ] - }, - "model_reasoning_effort": { - "anyOf": [ - { - "$ref": "#/definitions/ReasoningEffort" - }, - { - "type": "null" - } - ] - }, - "model_reasoning_summary": { - "anyOf": [ - { - "$ref": "#/definitions/ReasoningSummary" - }, - { - "type": "null" - } - ] - }, - "model_verbosity": { - "anyOf": [ - { - "$ref": "#/definitions/Verbosity" - }, - { - "type": "null" - } - ] - }, - "service_tier": { - "type": [ - "string", - "null" - ] - }, - "tools": { - "anyOf": [ - { - "$ref": "#/definitions/ToolsV2" - }, - { - "type": "null" - } - ] - }, - "web_search": { - "anyOf": [ - { - "$ref": "#/definitions/WebSearchMode" - }, - { - "type": "null" - } - ] - } - }, - "type": "object" - }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/Config.ts b/codex-rs/app-server-protocol/schema/typescript/v2/Config.ts index 29eae9877419..cc15fb4e720b 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/Config.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/Config.ts @@ -12,7 +12,6 @@ import type { AnalyticsConfig } from "./AnalyticsConfig"; import type { ApprovalsReviewer } from "./ApprovalsReviewer"; import type { AskForApproval } from "./AskForApproval"; import type { ForcedChatgptWorkspaceIds } from "./ForcedChatgptWorkspaceIds"; -import type { ProfileV2 } from "./ProfileV2"; import type { SandboxMode } from "./SandboxMode"; import type { SandboxWorkspaceWrite } from "./SandboxWorkspaceWrite"; import type { ToolsV2 } from "./ToolsV2"; @@ -21,4 +20,4 @@ export type Config = {model: string | null, review_model: string | null, model_c * [UNSTABLE] Optional default for where approval requests are routed for * review. */ -approvals_reviewer: ApprovalsReviewer | null, sandbox_mode: SandboxMode | null, sandbox_workspace_write: SandboxWorkspaceWrite | null, forced_chatgpt_workspace_id: ForcedChatgptWorkspaceIds | null, forced_login_method: ForcedLoginMethod | null, web_search: WebSearchMode | null, tools: ToolsV2 | null, profile: string | null, profiles: { [key in string]?: ProfileV2 }, instructions: string | null, developer_instructions: string | null, compact_prompt: string | null, model_reasoning_effort: ReasoningEffort | null, model_reasoning_summary: ReasoningSummary | null, model_verbosity: Verbosity | null, service_tier: string | null, analytics: AnalyticsConfig | null, desktop: { [key in string]?: JsonValue } | null} & ({ [key in string]?: number | string | boolean | Array | { [key in string]?: JsonValue } | null }); +approvals_reviewer: ApprovalsReviewer | null, sandbox_mode: SandboxMode | null, sandbox_workspace_write: SandboxWorkspaceWrite | null, forced_chatgpt_workspace_id: ForcedChatgptWorkspaceIds | null, forced_login_method: ForcedLoginMethod | null, web_search: WebSearchMode | null, tools: ToolsV2 | null, instructions: string | null, developer_instructions: string | null, compact_prompt: string | null, model_reasoning_effort: ReasoningEffort | null, model_reasoning_summary: ReasoningSummary | null, model_verbosity: Verbosity | null, service_tier: string | null, analytics: AnalyticsConfig | null, desktop: { [key in string]?: JsonValue } | null} & ({ [key in string]?: number | string | boolean | Array | { [key in string]?: JsonValue } | null }); diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ProfileV2.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ProfileV2.ts deleted file mode 100644 index d05038701c83..000000000000 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ProfileV2.ts +++ /dev/null @@ -1,18 +0,0 @@ -// 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 { ReasoningEffort } from "../ReasoningEffort"; -import type { ReasoningSummary } from "../ReasoningSummary"; -import type { Verbosity } from "../Verbosity"; -import type { WebSearchMode } from "../WebSearchMode"; -import type { JsonValue } from "../serde_json/JsonValue"; -import type { ApprovalsReviewer } from "./ApprovalsReviewer"; -import type { AskForApproval } from "./AskForApproval"; -import type { ToolsV2 } from "./ToolsV2"; - -export type ProfileV2 = {model: string | null, model_provider: string | null, approval_policy: AskForApproval | null, /** - * [UNSTABLE] Optional profile-level override for where approval requests - * are routed for review. If omitted, the enclosing config default is - * used. - */ -approvals_reviewer: ApprovalsReviewer | null, service_tier: string | null, model_reasoning_effort: ReasoningEffort | null, model_reasoning_summary: ReasoningSummary | null, model_verbosity: Verbosity | null, web_search: WebSearchMode | null, tools: ToolsV2 | null, chatgpt_base_url: string | null} & ({ [key in string]?: number | string | boolean | Array | { [key in string]?: JsonValue } | null }); 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 d5ae15e8e279..5b4f2ed2831a 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -308,7 +308,6 @@ export type { ProcessExitedNotification } from "./ProcessExitedNotification"; export type { ProcessOutputDeltaNotification } from "./ProcessOutputDeltaNotification"; export type { ProcessOutputStream } from "./ProcessOutputStream"; export type { ProcessTerminalSize } from "./ProcessTerminalSize"; -export type { ProfileV2 } from "./ProfileV2"; export type { RateLimitReachedType } from "./RateLimitReachedType"; export type { RateLimitSnapshot } from "./RateLimitSnapshot"; export type { RateLimitWindow } from "./RateLimitWindow"; diff --git a/codex-rs/app-server-protocol/src/lib.rs b/codex-rs/app-server-protocol/src/lib.rs index 2fcf54f4bee8..f6d7670e10a2 100644 --- a/codex-rs/app-server-protocol/src/lib.rs +++ b/codex-rs/app-server-protocol/src/lib.rs @@ -36,7 +36,6 @@ pub use protocol::v1::InitializeParams; pub use protocol::v1::InitializeResponse; pub use protocol::v1::InterruptConversationResponse; pub use protocol::v1::LoginApiKeyParams; -pub use protocol::v1::Profile; pub use protocol::v1::SandboxSettings; pub use protocol::v1::Tools; pub use protocol::v1::UserSavedConfig; diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index 3c45c20b8fc2..f83674d4c37d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -209,20 +209,6 @@ pub struct UserSavedConfig { pub model_reasoning_summary: Option, pub model_verbosity: Option, pub tools: Option, - pub profile: Option, - pub profiles: HashMap, -} - -#[derive(Deserialize, Debug, Clone, PartialEq, Serialize, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -pub struct Profile { - pub model: Option, - pub model_provider: Option, - pub approval_policy: Option, - pub model_reasoning_effort: Option, - pub model_reasoning_summary: Option, - pub model_verbosity: Option, - pub chatgpt_base_url: Option, } #[derive(Deserialize, Debug, Clone, PartialEq, Serialize, JsonSchema, TS)] diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index 16f9bf154b18..6f7dd3661bf4 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -133,30 +133,6 @@ pub struct ToolsV2 { pub web_search: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)] -#[serde(rename_all = "snake_case")] -#[ts(export_to = "v2/")] -pub struct ProfileV2 { - pub model: Option, - pub model_provider: Option, - #[experimental(nested)] - pub approval_policy: Option, - /// [UNSTABLE] Optional profile-level override for where approval requests - /// are routed for review. If omitted, the enclosing config default is - /// used. - #[experimental("config/read.approvalsReviewer")] - pub approvals_reviewer: Option, - pub service_tier: Option, - pub model_reasoning_effort: Option, - pub model_reasoning_summary: Option, - pub model_verbosity: Option, - pub web_search: Option, - pub tools: Option, - pub chatgpt_base_url: Option, - #[serde(default, flatten)] - pub additional: HashMap, -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] #[ts(export_to = "v2/")] @@ -266,10 +242,6 @@ pub struct Config { pub forced_login_method: Option, pub web_search: Option, pub tools: Option, - pub profile: Option, - #[experimental(nested)] - #[serde(default)] - pub profiles: HashMap, pub instructions: Option, pub developer_instructions: Option, pub compact_prompt: Option, @@ -282,10 +254,22 @@ pub struct Config { #[serde(default)] pub apps: Option, pub desktop: Option>, - #[serde(default, flatten)] + #[serde(default, flatten, deserialize_with = "deserialize_config_additional")] pub additional: HashMap, } +fn deserialize_config_additional<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let mut additional = HashMap::::deserialize(deserializer)?; + additional.remove("profile"); + additional.remove("profiles"); + Ok(additional) +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index bbe6bec3f2d5..7112e9eb1396 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -1505,32 +1505,6 @@ fn ask_for_approval_granular_is_marked_experimental() { ); } -#[test] -fn profile_v2_granular_approval_policy_is_marked_experimental() { - let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&ProfileV2 { - model: None, - model_provider: None, - approval_policy: Some(AskForApproval::Granular { - sandbox_approval: true, - rules: false, - skill_approval: false, - request_permissions: true, - mcp_elicitations: false, - }), - approvals_reviewer: None, - service_tier: None, - model_reasoning_effort: None, - model_reasoning_summary: None, - model_verbosity: None, - web_search: None, - tools: None, - chatgpt_base_url: None, - additional: HashMap::new(), - }); - - assert_eq!(reason, Some("askForApproval.granular")); -} - #[test] fn config_granular_approval_policy_is_marked_experimental() { let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&Config { @@ -1554,8 +1528,6 @@ fn config_granular_approval_policy_is_marked_experimental() { forced_login_method: None, web_search: None, tools: None, - profile: None, - profiles: HashMap::new(), instructions: None, developer_instructions: None, compact_prompt: None, @@ -1589,116 +1561,6 @@ fn config_approvals_reviewer_is_marked_experimental() { forced_login_method: None, web_search: None, tools: None, - profile: None, - profiles: HashMap::new(), - instructions: None, - developer_instructions: None, - compact_prompt: None, - model_reasoning_effort: None, - model_reasoning_summary: None, - model_verbosity: None, - service_tier: None, - analytics: None, - apps: None, - desktop: None, - additional: HashMap::new(), - }); - - assert_eq!(reason, Some("config/read.approvalsReviewer")); -} - -#[test] -fn config_nested_profile_granular_approval_policy_is_marked_experimental() { - let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&Config { - model: None, - review_model: None, - model_context_window: None, - model_auto_compact_token_limit: None, - model_auto_compact_token_limit_scope: None, - model_provider: None, - approval_policy: None, - approvals_reviewer: None, - sandbox_mode: None, - sandbox_workspace_write: None, - forced_chatgpt_workspace_id: None, - forced_login_method: None, - web_search: None, - tools: None, - profile: None, - profiles: HashMap::from([( - "default".to_string(), - ProfileV2 { - model: None, - model_provider: None, - approval_policy: Some(AskForApproval::Granular { - sandbox_approval: true, - rules: false, - skill_approval: false, - request_permissions: false, - mcp_elicitations: true, - }), - approvals_reviewer: None, - service_tier: None, - model_reasoning_effort: None, - model_reasoning_summary: None, - model_verbosity: None, - web_search: None, - tools: None, - chatgpt_base_url: None, - additional: HashMap::new(), - }, - )]), - instructions: None, - developer_instructions: None, - compact_prompt: None, - model_reasoning_effort: None, - model_reasoning_summary: None, - model_verbosity: None, - service_tier: None, - analytics: None, - apps: None, - desktop: None, - additional: HashMap::new(), - }); - - assert_eq!(reason, Some("askForApproval.granular")); -} - -#[test] -fn config_nested_profile_approvals_reviewer_is_marked_experimental() { - let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&Config { - model: None, - review_model: None, - model_context_window: None, - model_auto_compact_token_limit: None, - model_auto_compact_token_limit_scope: None, - model_provider: None, - approval_policy: None, - approvals_reviewer: None, - sandbox_mode: None, - sandbox_workspace_write: None, - forced_chatgpt_workspace_id: None, - forced_login_method: None, - web_search: None, - tools: None, - profile: None, - profiles: HashMap::from([( - "default".to_string(), - ProfileV2 { - model: None, - model_provider: None, - approval_policy: None, - approvals_reviewer: Some(ApprovalsReviewer::AutoReview), - service_tier: None, - model_reasoning_effort: None, - model_reasoning_summary: None, - model_verbosity: None, - web_search: None, - tools: None, - chatgpt_base_url: None, - additional: HashMap::new(), - }, - )]), instructions: None, developer_instructions: None, compact_prompt: None, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2ceffc86fec4..72a2dbc09ad6 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1931,7 +1931,7 @@ reason up through the containing type: ```rust #[derive(ExperimentalApi)] -struct ProfileV2 { +struct Config { #[experimental(nested)] approval_policy: Option, } diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index 2f3cc5ef97ed..214a3ef84759 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -238,12 +238,22 @@ impl ConfigManager { let segments = parse_key_path(&key_path).map_err(|message| { ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) })?; - if matches!(segments.as_slice(), [segment] if segment == "profile") && !value.is_null() - { - return Err(ConfigManagerError::write( - ConfigWriteErrorCode::ConfigValidationError, - "`profile` is a legacy config selector and can no longer be written; use `--profile ` with `.config.toml` instead", - )); + if !value.is_null() { + match segments.as_slice() { + [segment] if segment == "profile" => { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + "`profile` is a legacy config selector and can no longer be written; use `--profile ` with `.config.toml` instead", + )); + } + [segment, ..] if segment == "profiles" => { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + "`profiles` contains legacy config profile tables and can no longer be written; use `--profile ` with `.config.toml` instead", + )); + } + _ => {} + } } let original_value = value_at_path(&user_config, &segments).cloned(); let parsed_value = parse_value(value).map_err(|message| { diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index be35a1977eda..8c178d6680f3 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -162,6 +162,38 @@ async fn write_value_rejects_legacy_profile_selector() -> Result<()> { Ok(()) } +#[tokio::test] +async fn write_value_rejects_legacy_profile_table() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + let path = tmp.path().join(CONFIG_TOML_FILE); + std::fs::write(&path, "")?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + let error = service + .write_value(ConfigValueWriteParams { + file_path: Some(path.display().to_string()), + key_path: "profiles.work.model".to_string(), + value: serde_json::json!("gpt-work"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect_err("legacy profile table write should fail"); + + assert_eq!( + error.write_error_code(), + Some(ConfigWriteErrorCode::ConfigValidationError) + ); + assert!( + error + .to_string() + .contains("`profiles` contains legacy config profile tables"), + "{error}" + ); + assert_eq!(std::fs::read_to_string(&path)?, ""); + Ok(()) +} + #[tokio::test] async fn batch_write_rejects_legacy_profile_selector() -> Result<()> { let tmp = tempdir().expect("tempdir"); @@ -204,6 +236,35 @@ async fn batch_write_rejects_legacy_profile_selector() -> Result<()> { Ok(()) } +#[tokio::test] +async fn write_value_clears_legacy_profile_table() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + let path = tmp.path().join(CONFIG_TOML_FILE); + std::fs::write( + &path, + r#"[profiles.work] +model = "gpt-work" +"#, + )?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + let response = service + .write_value(ConfigValueWriteParams { + file_path: Some(path.display().to_string()), + key_path: "profiles.work".to_string(), + value: serde_json::Value::Null, + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("legacy profile table clear should succeed"); + + assert_eq!(response.status, WriteStatus::Ok); + let config: toml::Value = toml::from_str(&std::fs::read_to_string(&path)?)?; + assert!(config.get("profiles").is_none()); + Ok(()) +} + #[tokio::test] async fn write_value_supports_nested_app_paths() -> Result<()> { let tmp = tempdir().expect("tempdir"); @@ -308,6 +369,33 @@ async fn write_value_supports_custom_mcp_server_default_tool_approval_mode() -> Ok(()) } +#[tokio::test] +async fn read_omits_legacy_profile_config() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#"profile = "work" + +[profiles.work] +model = "gpt-work" +"#, + )?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + let read = service + .read(ConfigReadParams { + include_layers: false, + cwd: None, + }) + .await + .expect("config read succeeds"); + + let config = serde_json::to_value(read.config).expect("config serializes"); + assert!(config.get("profile").is_none()); + assert!(config.get("profiles").is_none()); + Ok(()) +} + #[tokio::test] async fn read_includes_origins_and_layers() { let tmp = tempdir().expect("tempdir"); @@ -712,52 +800,6 @@ async fn write_value_rejects_feature_requirement_conflict() { ); } -#[tokio::test] -async fn write_value_rejects_profile_feature_requirement_conflict() { - let tmp = tempdir().expect("tempdir"); - std::fs::write(tmp.path().join(CONFIG_TOML_FILE), "").unwrap(); - - let service = ConfigManager::new_for_tests( - tmp.path().to_path_buf(), - vec![], - LoaderOverrides::without_managed_config_for_tests(), - CloudRequirementsLoader::new(async { - Ok(Some(ConfigRequirementsToml { - feature_requirements: Some(FeatureRequirementsToml { - entries: BTreeMap::from([("personality".to_string(), true)]), - }), - ..Default::default() - })) - }), - ); - - let error = service - .write_value(ConfigValueWriteParams { - file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), - key_path: "profiles.enterprise.features.personality".to_string(), - value: serde_json::json!(false), - merge_strategy: MergeStrategy::Replace, - expected_version: None, - }) - .await - .expect_err("conflicting profile feature write should fail"); - - assert_eq!( - error.write_error_code(), - Some(ConfigWriteErrorCode::ConfigValidationError) - ); - assert!( - error.to_string().contains( - "invalid value for `features`: `profiles.enterprise.features.personality=false`" - ), - "{error}" - ); - assert_eq!( - std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap(), - "" - ); -} - #[tokio::test] async fn read_reports_managed_overrides_user_and_session_flags() { let tmp = tempdir().expect("tempdir"); diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index 56c4d0b7d140..0bfd8ec876e6 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -894,19 +894,14 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn config_batch_write_preserves_dotted_profile_names() -> Result<()> { +async fn config_batch_write_rejects_legacy_profile_tables() -> Result<()> { let tmp_dir = TempDir::new()?; let codex_home = tmp_dir.path().canonicalize()?; write_config( &tmp_dir, r#" -profile = "team.prod" - [profiles."team.prod"] model = "gpt-5.3-spark" - -[profiles.team.prod] -model = "should-stay-put" "#, )?; @@ -932,28 +927,30 @@ model = "should-stay-put" reload_user_config: false, }) .await?; - let batch_resp: JSONRPCResponse = timeout( + let err: JSONRPCError = timeout( DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(batch_id)), + mcp.read_stream_until_error_message(RequestId::Integer(batch_id)), ) .await??; - let batch_write: ConfigWriteResponse = to_response(batch_resp)?; - assert_eq!(batch_write.status, WriteStatus::Ok); + let code = err + .error + .data + .as_ref() + .and_then(|data| data.get("config_write_error_code")) + .and_then(|value| value.as_str()); + assert_eq!(code, Some("configValidationError")); + assert!( + err.error.message.contains("`profiles`"), + "unexpected error: {err:?}" + ); let config: toml::Value = toml::from_str(&std::fs::read_to_string(codex_home.join("config.toml"))?)?; assert_eq!( config["profiles"]["team.prod"]["model"].as_str(), - Some("gpt-5.5") - ); - assert_eq!( - config["profiles"]["team"]["prod"]["model"].as_str(), - Some("should-stay-put") - ); - assert_eq!( - config["items"]["sample@catalog"]["enabled"].as_bool(), - Some(true) + Some("gpt-5.3-spark") ); + assert_eq!(config.get("items"), None); Ok(()) } diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index a4d384b5f39e..7b0458e087a7 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -27,9 +27,6 @@ use crate::types::ToolSuggestConfig; use crate::types::Tui; use crate::types::UriBasedFileOpener; use crate::types::WindowsToml; -use codex_app_server_protocol::ForcedChatgptWorkspaceIds as ApiForcedChatgptWorkspaceIds; -use codex_app_server_protocol::Tools; -use codex_app_server_protocol::UserSavedConfig; use codex_features::FeaturesToml; use codex_model_provider_info::AMAZON_BEDROCK_PROVIDER_ID; use codex_model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID; @@ -105,13 +102,6 @@ impl ForcedChatgptWorkspaceIds { Self::Multiple(values) => values, } } - - pub fn into_api(self) -> ApiForcedChatgptWorkspaceIds { - match self { - Self::Single(value) => ApiForcedChatgptWorkspaceIds::Single(value), - Self::Multiple(values) => ApiForcedChatgptWorkspaceIds::Multiple(values), - } - } } impl<'de> Deserialize<'de> for ForcedChatgptWorkspaceIds { @@ -552,33 +542,6 @@ pub struct AutoReviewToml { pub policy: Option, } -impl From for UserSavedConfig { - fn from(config_toml: ConfigToml) -> Self { - let profiles = config_toml - .profiles - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(); - - Self { - approval_policy: config_toml.approval_policy, - sandbox_mode: config_toml.sandbox_mode, - sandbox_settings: config_toml.sandbox_workspace_write.map(From::from), - forced_chatgpt_workspace_id: config_toml - .forced_chatgpt_workspace_id - .map(ForcedChatgptWorkspaceIds::into_api), - forced_login_method: config_toml.forced_login_method, - model: config_toml.model, - model_reasoning_effort: config_toml.model_reasoning_effort, - model_reasoning_summary: config_toml.model_reasoning_summary, - model_verbosity: config_toml.model_verbosity, - tools: config_toml.tools.map(From::from), - profile: config_toml.profile, - profiles, - } - } -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct ProjectConfig { @@ -729,14 +692,6 @@ pub struct AgentRoleToml { pub nickname_candidates: Option>, } -impl From for Tools { - fn from(tools_toml: ToolsToml) -> Self { - Self { - web_search: tools_toml.web_search.is_some().then_some(true), - } - } -} - #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct GhostSnapshotToml { diff --git a/codex-rs/config/src/profile_toml.rs b/codex-rs/config/src/profile_toml.rs index 5f4c8d62f910..e7cddd3d679f 100644 --- a/codex-rs/config/src/profile_toml.rs +++ b/codex-rs/config/src/profile_toml.rs @@ -81,17 +81,3 @@ pub struct ProfileTui { #[serde(default)] pub session_picker_view: Option, } - -impl From for codex_app_server_protocol::Profile { - fn from(config_profile: ConfigProfile) -> Self { - Self { - model: config_profile.model, - model_provider: config_profile.model_provider, - approval_policy: config_profile.approval_policy, - model_reasoning_effort: config_profile.model_reasoning_effort, - model_reasoning_summary: config_profile.model_reasoning_summary, - model_verbosity: config_profile.model_verbosity, - chatgpt_base_url: config_profile.chatgpt_base_url, - } - } -} From dfd29bba8de691af566974742827c626b55c2235 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 22 May 2026 13:20:19 +0100 Subject: [PATCH 2/5] app-server: reject legacy profile reads --- .../src/protocol/v2/config.rs | 14 +--- .../app-server/src/config_manager_service.rs | 65 ++++++++++---- .../src/config_manager_service_tests.rs | 84 +++++++------------ 3 files changed, 78 insertions(+), 85 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index 6f7dd3661bf4..bef794ced366 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -254,22 +254,10 @@ pub struct Config { #[serde(default)] pub apps: Option, pub desktop: Option>, - #[serde(default, flatten, deserialize_with = "deserialize_config_additional")] + #[serde(default, flatten)] pub additional: HashMap, } -fn deserialize_config_additional<'de, D>( - deserializer: D, -) -> Result, D::Error> -where - D: serde::Deserializer<'de>, -{ - let mut additional = HashMap::::deserialize(deserializer)?; - additional.remove("profile"); - additional.remove("profiles"); - Ok(additional) -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index 214a3ef84759..e27730810fdd 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -65,6 +65,9 @@ pub(crate) enum ConfigManagerError { source: toml::de::Error, }, + #[error("invalid configuration: {message}")] + InvalidConfig { message: String }, + #[error("{context}: {source}")] Anyhow { context: &'static str, @@ -93,6 +96,12 @@ impl ConfigManagerError { Self::Toml { context, source } } + fn invalid_config(message: impl Into) -> Self { + Self::InvalidConfig { + message: message.into(), + } + } + fn anyhow(context: &'static str, source: anyhow::Error) -> Self { Self::Anyhow { context, source } } @@ -124,8 +133,9 @@ impl ConfigManager { })?, }; - let effective = layers.effective_config(); + reject_legacy_profile_read(&layers)?; + let effective = layers.effective_config(); let effective_config_toml: ConfigToml = effective .try_into() .map_err(|err| ConfigManagerError::toml("invalid configuration", err))?; @@ -239,21 +249,7 @@ impl ConfigManager { ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) })?; if !value.is_null() { - match segments.as_slice() { - [segment] if segment == "profile" => { - return Err(ConfigManagerError::write( - ConfigWriteErrorCode::ConfigValidationError, - "`profile` is a legacy config selector and can no longer be written; use `--profile ` with `.config.toml` instead", - )); - } - [segment, ..] if segment == "profiles" => { - return Err(ConfigManagerError::write( - ConfigWriteErrorCode::ConfigValidationError, - "`profiles` contains legacy config profile tables and can no longer be written; use `--profile ` with `.config.toml` instead", - )); - } - _ => {} - } + reject_legacy_profile_write(&segments)?; } let original_value = value_at_path(&user_config, &segments).cloned(); let parsed_value = parse_value(value).map_err(|message| { @@ -399,6 +395,43 @@ async fn create_empty_user_layer( )) } +fn reject_legacy_profile_read(layers: &ConfigLayerStack) -> Result<(), ConfigManagerError> { + let contains_legacy_profiles = layers + .get_layers( + ConfigLayerStackOrdering::HighestPrecedenceFirst, + /*include_disabled*/ true, + ) + .iter() + .any(|layer| legacy_profile_keys_present(&layer.config)); + if !contains_legacy_profiles { + return Ok(()); + } + + Err(ConfigManagerError::invalid_config( + "legacy config profiles are no longer supported by config/read; use `--profile ` with `.config.toml` instead", + )) +} + +fn legacy_profile_keys_present(config: &TomlValue) -> bool { + config + .as_table() + .is_some_and(|table| table.contains_key("profile") || table.contains_key("profiles")) +} + +fn reject_legacy_profile_write(segments: &[String]) -> Result<(), ConfigManagerError> { + match segments { + [segment] if segment == "profile" => Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + "`profile` is a legacy config selector and can no longer be written; use `--profile ` with `.config.toml` instead", + )), + [segment, ..] if segment == "profiles" => Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + "`profiles` contains legacy config profile tables and can no longer be written; use `--profile ` with `.config.toml` instead", + )), + _ => Ok(()), + } +} + async fn write_empty_user_config(write_path: PathBuf) -> Result<(), ConfigManagerError> { task::spawn_blocking(move || write_atomically(&write_path, "")) .await diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index 8c178d6680f3..444e7c20c802 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -162,6 +162,34 @@ async fn write_value_rejects_legacy_profile_selector() -> Result<()> { Ok(()) } +#[tokio::test] +async fn read_rejects_legacy_profile_tables() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#"[profiles.work] +model = "gpt-work" +"#, + )?; + + let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); + let error = service + .read(ConfigReadParams { + include_layers: true, + cwd: None, + }) + .await + .expect_err("legacy profile table read should fail"); + + assert!( + error + .to_string() + .contains("legacy config profiles are no longer supported by config/read"), + "{error}" + ); + Ok(()) +} + #[tokio::test] async fn write_value_rejects_legacy_profile_table() -> Result<()> { let tmp = tempdir().expect("tempdir"); @@ -236,35 +264,6 @@ async fn batch_write_rejects_legacy_profile_selector() -> Result<()> { Ok(()) } -#[tokio::test] -async fn write_value_clears_legacy_profile_table() -> Result<()> { - let tmp = tempdir().expect("tempdir"); - let path = tmp.path().join(CONFIG_TOML_FILE); - std::fs::write( - &path, - r#"[profiles.work] -model = "gpt-work" -"#, - )?; - - let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); - let response = service - .write_value(ConfigValueWriteParams { - file_path: Some(path.display().to_string()), - key_path: "profiles.work".to_string(), - value: serde_json::Value::Null, - merge_strategy: MergeStrategy::Replace, - expected_version: None, - }) - .await - .expect("legacy profile table clear should succeed"); - - assert_eq!(response.status, WriteStatus::Ok); - let config: toml::Value = toml::from_str(&std::fs::read_to_string(&path)?)?; - assert!(config.get("profiles").is_none()); - Ok(()) -} - #[tokio::test] async fn write_value_supports_nested_app_paths() -> Result<()> { let tmp = tempdir().expect("tempdir"); @@ -369,33 +368,6 @@ async fn write_value_supports_custom_mcp_server_default_tool_approval_mode() -> Ok(()) } -#[tokio::test] -async fn read_omits_legacy_profile_config() -> Result<()> { - let tmp = tempdir().expect("tempdir"); - std::fs::write( - tmp.path().join(CONFIG_TOML_FILE), - r#"profile = "work" - -[profiles.work] -model = "gpt-work" -"#, - )?; - - let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); - let read = service - .read(ConfigReadParams { - include_layers: false, - cwd: None, - }) - .await - .expect("config read succeeds"); - - let config = serde_json::to_value(read.config).expect("config serializes"); - assert!(config.get("profile").is_none()); - assert!(config.get("profiles").is_none()); - Ok(()) -} - #[tokio::test] async fn read_includes_origins_and_layers() { let tmp = tempdir().expect("tempdir"); From a9c2119c0a95b145bed0522e6a9170b70f281324 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 22 May 2026 13:41:02 +0100 Subject: [PATCH 3/5] app-server: allow unrelated legacy profiles --- .../app-server/src/config_manager_service.rs | 70 ++++++++++++++++--- .../src/config_manager_service_tests.rs | 53 ++++++++++++++ 2 files changed, 115 insertions(+), 8 deletions(-) diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index e27730810fdd..4fc1da2fd8f4 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -135,7 +135,8 @@ impl ConfigManager { reject_legacy_profile_read(&layers)?; - let effective = layers.effective_config(); + let mut effective = layers.effective_config(); + strip_legacy_profile_keys(&mut effective); let effective_config_toml: ConfigToml = effective .try_into() .map_err(|err| ConfigManagerError::toml("invalid configuration", err))?; @@ -147,7 +148,7 @@ impl ConfigManager { Ok(ConfigReadResponse { config, - origins: layers.origins(), + origins: origins_without_legacy_profiles(&layers), layers: params.include_layers.then(|| { layers .get_layers( @@ -155,7 +156,11 @@ impl ConfigManager { /*include_disabled*/ true, ) .iter() - .map(|layer| layer.as_layer()) + .map(|layer| { + let mut layer = layer.as_layer(); + strip_json_legacy_profile_keys(&mut layer.config); + layer + }) .collect() }), }) @@ -396,13 +401,28 @@ async fn create_empty_user_layer( } fn reject_legacy_profile_read(layers: &ConfigLayerStack) -> Result<(), ConfigManagerError> { + let profile_v2_active = layers + .get_user_layers( + ConfigLayerStackOrdering::HighestPrecedenceFirst, + /*include_disabled*/ false, + ) + .iter() + .any(|layer| { + matches!( + &layer.name, + ConfigLayerSource::User { + profile: Some(_), + .. + } + ) + }); let contains_legacy_profiles = layers .get_layers( ConfigLayerStackOrdering::HighestPrecedenceFirst, /*include_disabled*/ true, ) .iter() - .any(|layer| legacy_profile_keys_present(&layer.config)); + .any(|layer| legacy_profile_keys_rejected(layer, profile_v2_active)); if !contains_legacy_profiles { return Ok(()); } @@ -412,10 +432,44 @@ fn reject_legacy_profile_read(layers: &ConfigLayerStack) -> Result<(), ConfigMan )) } -fn legacy_profile_keys_present(config: &TomlValue) -> bool { - config - .as_table() - .is_some_and(|table| table.contains_key("profile") || table.contains_key("profiles")) +fn legacy_profile_keys_rejected(layer: &ConfigLayerEntry, profile_v2_active: bool) -> bool { + let Some(config) = layer.config.as_table() else { + return false; + }; + if config.contains_key("profile") { + return true; + } + if !config.contains_key("profiles") { + return false; + } + + !profile_v2_active || !matches!(&layer.name, ConfigLayerSource::User { profile: None, .. }) +} + +fn strip_legacy_profile_keys(config: &mut TomlValue) { + if let Some(config) = config.as_table_mut() { + config.remove("profile"); + config.remove("profiles"); + } +} + +fn strip_json_legacy_profile_keys(config: &mut JsonValue) { + if let Some(config) = config.as_object_mut() { + config.remove("profile"); + config.remove("profiles"); + } +} + +fn origins_without_legacy_profiles( + layers: &ConfigLayerStack, +) -> std::collections::HashMap { + let mut origins = layers.origins(); + origins.retain(|path, _| !legacy_profile_origin_path(path)); + origins +} + +fn legacy_profile_origin_path(path: &str) -> bool { + path == "profile" || path == "profiles" || path.starts_with("profiles.") } fn reject_legacy_profile_write(segments: &[String]) -> Result<(), ConfigManagerError> { diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index 444e7c20c802..96d1e2ebcf94 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -190,6 +190,59 @@ model = "gpt-work" Ok(()) } +#[tokio::test] +async fn read_allows_unrelated_legacy_profiles_with_selected_user_profile() -> Result<()> { + let tmp = tempdir().expect("tempdir"); + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#"model = "gpt-main" + +[profiles.dev] +model = "gpt-dev" +"#, + )?; + let selected_path = tmp.path().join("work.config.toml"); + std::fs::write(&selected_path, "model = \"gpt-work\"\n")?; + + let mut loader_overrides = + LoaderOverrides::with_managed_config_path_for_tests(tmp.path().join("managed_config.toml")); + loader_overrides.user_config_path = + Some(AbsolutePathBuf::from_absolute_path(&selected_path).expect("selected config path")); + loader_overrides.user_config_profile = Some("work".parse().expect("profile-v2 name")); + let service = ConfigManager::new_for_tests( + tmp.path().to_path_buf(), + vec![], + loader_overrides, + CloudRequirementsLoader::default(), + ); + + let read = service + .read(ConfigReadParams { + include_layers: true, + cwd: None, + }) + .await + .expect("unrelated legacy profile table should not block config read"); + + assert_eq!(read.config.model.as_deref(), Some("gpt-work")); + assert!(!read.config.additional.contains_key("profiles")); + assert!( + read.origins + .keys() + .all(|path| !path.starts_with("profiles.")), + "origins still expose legacy profiles: {:?}", + read.origins + ); + assert!( + read.layers + .expect("layers present") + .iter() + .all(|layer| layer.config.get("profiles").is_none()), + "layers still expose legacy profiles" + ); + Ok(()) +} + #[tokio::test] async fn write_value_rejects_legacy_profile_table() -> Result<()> { let tmp = tempdir().expect("tempdir"); From 79fca5163add20ebed9cf09aa26dcf3596a0f6c6 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 22 May 2026 13:55:03 +0100 Subject: [PATCH 4/5] app-server: simplify legacy profile cleanup --- .../app-server/src/config_manager_service.rs | 125 +++--------------- .../src/config_manager_service_tests.rs | 81 ------------ 2 files changed, 18 insertions(+), 188 deletions(-) diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index 4fc1da2fd8f4..2eea946f77dd 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -1,7 +1,6 @@ use crate::config_manager::ConfigManager; use codex_app_server_protocol::Config as ApiConfig; use codex_app_server_protocol::ConfigBatchWriteParams; -use codex_app_server_protocol::ConfigLayerMetadata; use codex_app_server_protocol::ConfigLayerSource; use codex_app_server_protocol::ConfigReadParams; use codex_app_server_protocol::ConfigReadResponse; @@ -65,9 +64,6 @@ pub(crate) enum ConfigManagerError { source: toml::de::Error, }, - #[error("invalid configuration: {message}")] - InvalidConfig { message: String }, - #[error("{context}: {source}")] Anyhow { context: &'static str, @@ -96,12 +92,6 @@ impl ConfigManagerError { Self::Toml { context, source } } - fn invalid_config(message: impl Into) -> Self { - Self::InvalidConfig { - message: message.into(), - } - } - fn anyhow(context: &'static str, source: anyhow::Error) -> Self { Self::Anyhow { context, source } } @@ -133,10 +123,7 @@ impl ConfigManager { })?, }; - reject_legacy_profile_read(&layers)?; - - let mut effective = layers.effective_config(); - strip_legacy_profile_keys(&mut effective); + let effective = layers.effective_config(); let effective_config_toml: ConfigToml = effective .try_into() .map_err(|err| ConfigManagerError::toml("invalid configuration", err))?; @@ -148,7 +135,7 @@ impl ConfigManager { Ok(ConfigReadResponse { config, - origins: origins_without_legacy_profiles(&layers), + origins: layers.origins(), layers: params.include_layers.then(|| { layers .get_layers( @@ -156,11 +143,7 @@ impl ConfigManager { /*include_disabled*/ true, ) .iter() - .map(|layer| { - let mut layer = layer.as_layer(); - strip_json_legacy_profile_keys(&mut layer.config); - layer - }) + .map(|layer| layer.as_layer()) .collect() }), }) @@ -254,7 +237,21 @@ impl ConfigManager { ConfigManagerError::write(ConfigWriteErrorCode::ConfigValidationError, message) })?; if !value.is_null() { - reject_legacy_profile_write(&segments)?; + match segments.as_slice() { + [segment] if segment == "profile" => { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + "`profile` is a legacy config selector and can no longer be written; use `--profile ` with `.config.toml` instead", + )); + } + [segment, ..] if segment == "profiles" => { + return Err(ConfigManagerError::write( + ConfigWriteErrorCode::ConfigValidationError, + "`profiles` contains legacy config profile tables and can no longer be written; use `--profile ` with `.config.toml` instead", + )); + } + _ => {} + } } let original_value = value_at_path(&user_config, &segments).cloned(); let parsed_value = parse_value(value).map_err(|message| { @@ -400,92 +397,6 @@ async fn create_empty_user_layer( )) } -fn reject_legacy_profile_read(layers: &ConfigLayerStack) -> Result<(), ConfigManagerError> { - let profile_v2_active = layers - .get_user_layers( - ConfigLayerStackOrdering::HighestPrecedenceFirst, - /*include_disabled*/ false, - ) - .iter() - .any(|layer| { - matches!( - &layer.name, - ConfigLayerSource::User { - profile: Some(_), - .. - } - ) - }); - let contains_legacy_profiles = layers - .get_layers( - ConfigLayerStackOrdering::HighestPrecedenceFirst, - /*include_disabled*/ true, - ) - .iter() - .any(|layer| legacy_profile_keys_rejected(layer, profile_v2_active)); - if !contains_legacy_profiles { - return Ok(()); - } - - Err(ConfigManagerError::invalid_config( - "legacy config profiles are no longer supported by config/read; use `--profile ` with `.config.toml` instead", - )) -} - -fn legacy_profile_keys_rejected(layer: &ConfigLayerEntry, profile_v2_active: bool) -> bool { - let Some(config) = layer.config.as_table() else { - return false; - }; - if config.contains_key("profile") { - return true; - } - if !config.contains_key("profiles") { - return false; - } - - !profile_v2_active || !matches!(&layer.name, ConfigLayerSource::User { profile: None, .. }) -} - -fn strip_legacy_profile_keys(config: &mut TomlValue) { - if let Some(config) = config.as_table_mut() { - config.remove("profile"); - config.remove("profiles"); - } -} - -fn strip_json_legacy_profile_keys(config: &mut JsonValue) { - if let Some(config) = config.as_object_mut() { - config.remove("profile"); - config.remove("profiles"); - } -} - -fn origins_without_legacy_profiles( - layers: &ConfigLayerStack, -) -> std::collections::HashMap { - let mut origins = layers.origins(); - origins.retain(|path, _| !legacy_profile_origin_path(path)); - origins -} - -fn legacy_profile_origin_path(path: &str) -> bool { - path == "profile" || path == "profiles" || path.starts_with("profiles.") -} - -fn reject_legacy_profile_write(segments: &[String]) -> Result<(), ConfigManagerError> { - match segments { - [segment] if segment == "profile" => Err(ConfigManagerError::write( - ConfigWriteErrorCode::ConfigValidationError, - "`profile` is a legacy config selector and can no longer be written; use `--profile ` with `.config.toml` instead", - )), - [segment, ..] if segment == "profiles" => Err(ConfigManagerError::write( - ConfigWriteErrorCode::ConfigValidationError, - "`profiles` contains legacy config profile tables and can no longer be written; use `--profile ` with `.config.toml` instead", - )), - _ => Ok(()), - } -} - async fn write_empty_user_config(write_path: PathBuf) -> Result<(), ConfigManagerError> { task::spawn_blocking(move || write_atomically(&write_path, "")) .await diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index 96d1e2ebcf94..5c6e3de5a100 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -162,87 +162,6 @@ async fn write_value_rejects_legacy_profile_selector() -> Result<()> { Ok(()) } -#[tokio::test] -async fn read_rejects_legacy_profile_tables() -> Result<()> { - let tmp = tempdir().expect("tempdir"); - std::fs::write( - tmp.path().join(CONFIG_TOML_FILE), - r#"[profiles.work] -model = "gpt-work" -"#, - )?; - - let service = ConfigManager::without_managed_config_for_tests(tmp.path().to_path_buf()); - let error = service - .read(ConfigReadParams { - include_layers: true, - cwd: None, - }) - .await - .expect_err("legacy profile table read should fail"); - - assert!( - error - .to_string() - .contains("legacy config profiles are no longer supported by config/read"), - "{error}" - ); - Ok(()) -} - -#[tokio::test] -async fn read_allows_unrelated_legacy_profiles_with_selected_user_profile() -> Result<()> { - let tmp = tempdir().expect("tempdir"); - std::fs::write( - tmp.path().join(CONFIG_TOML_FILE), - r#"model = "gpt-main" - -[profiles.dev] -model = "gpt-dev" -"#, - )?; - let selected_path = tmp.path().join("work.config.toml"); - std::fs::write(&selected_path, "model = \"gpt-work\"\n")?; - - let mut loader_overrides = - LoaderOverrides::with_managed_config_path_for_tests(tmp.path().join("managed_config.toml")); - loader_overrides.user_config_path = - Some(AbsolutePathBuf::from_absolute_path(&selected_path).expect("selected config path")); - loader_overrides.user_config_profile = Some("work".parse().expect("profile-v2 name")); - let service = ConfigManager::new_for_tests( - tmp.path().to_path_buf(), - vec![], - loader_overrides, - CloudRequirementsLoader::default(), - ); - - let read = service - .read(ConfigReadParams { - include_layers: true, - cwd: None, - }) - .await - .expect("unrelated legacy profile table should not block config read"); - - assert_eq!(read.config.model.as_deref(), Some("gpt-work")); - assert!(!read.config.additional.contains_key("profiles")); - assert!( - read.origins - .keys() - .all(|path| !path.starts_with("profiles.")), - "origins still expose legacy profiles: {:?}", - read.origins - ); - assert!( - read.layers - .expect("layers present") - .iter() - .all(|layer| layer.config.get("profiles").is_none()), - "layers still expose legacy profiles" - ); - Ok(()) -} - #[tokio::test] async fn write_value_rejects_legacy_profile_table() -> Result<()> { let tmp = tempdir().expect("tempdir"); From 8bf58f8e830a29fae24bc0d6afe5a9ea5904c5c9 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 22 May 2026 13:59:59 +0100 Subject: [PATCH 5/5] app-server: restore config layer metadata import --- codex-rs/app-server/src/config_manager_service.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index 2eea946f77dd..d2465d32dd8e 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -1,6 +1,7 @@ use crate::config_manager::ConfigManager; use codex_app_server_protocol::Config as ApiConfig; use codex_app_server_protocol::ConfigBatchWriteParams; +use codex_app_server_protocol::ConfigLayerMetadata; use codex_app_server_protocol::ConfigLayerSource; use codex_app_server_protocol::ConfigReadParams; use codex_app_server_protocol::ConfigReadResponse;