From 96af42ad981515878895184cf93dca9b7c1ea486 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Sat, 31 Jan 2026 18:03:00 +0100 Subject: [PATCH] even further --- codex-rs/app-server-protocol/src/export.rs | 110 +++++++++++++----- .../src/protocol/common.rs | 24 ++-- .../app-server-protocol/src/protocol/v2.rs | 20 ++++ .../app-server/src/codex_message_processor.rs | 16 +++ .../app-server/tests/common/mcp_process.rs | 10 ++ .../tests/suite/v2/experimental_api.rs | 28 ++--- 6 files changed, 146 insertions(+), 62 deletions(-) diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index baeb935699..f47d23d911 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -245,7 +245,7 @@ fn filter_thread_start_params_ts( let mut content = fs::read_to_string(&path).with_context(|| format!("Failed to read {}", path.display()))?; - let Some((_eq_index, open_brace, close_brace)) = type_alias_brace_span(&content) else { + let Some((open_brace, close_brace)) = type_body_brace_span(&content) else { return Ok(()); }; let inner = &content[open_brace + 1..close_brace]; @@ -254,10 +254,11 @@ fn filter_thread_start_params_ts( .filter(|field| field.type_name == "ThreadStartParams") .map(|field| field.field_name) .collect(); - let fields = split_top_level(inner, ','); + let fields = split_top_level_multi(inner, &[',', ';']); let filtered_fields: Vec = fields .into_iter() .filter(|field| { + let field = strip_leading_block_comments(field); parse_property_name(field) .is_none_or(|name| !experimental_field_names.contains(name.as_str())) }) @@ -461,11 +462,21 @@ fn split_type_alias(content: &str) -> Option<(String, String, String)> { Some((prefix, body, suffix)) } -fn type_alias_brace_span(content: &str) -> Option<(usize, usize, usize)> { - let eq_index = content.find('=')?; - let after_eq = &content[eq_index + 1..]; - let (open_rel, close_rel) = find_top_level_brace_span(after_eq)?; - Some((eq_index, eq_index + 1 + open_rel, eq_index + 1 + close_rel)) +fn type_body_brace_span(content: &str) -> Option<(usize, usize)> { + if let Some(eq_index) = content.find('=') { + let after_eq = &content[eq_index + 1..]; + let (open_rel, close_rel) = find_top_level_brace_span(after_eq)?; + return Some((eq_index + 1 + open_rel, eq_index + 1 + close_rel)); + } + + const INTERFACE_MARKER: &str = "export interface"; + let interface_index = content.find(INTERFACE_MARKER)?; + let after_interface = &content[interface_index + INTERFACE_MARKER.len()..]; + let (open_rel, close_rel) = find_top_level_brace_span(after_interface)?; + Some(( + interface_index + INTERFACE_MARKER.len() + open_rel, + interface_index + INTERFACE_MARKER.len() + close_rel, + )) } fn find_top_level_brace_span(input: &str) -> Option<(usize, usize)> { @@ -488,11 +499,15 @@ fn find_top_level_brace_span(input: &str) -> Option<(usize, usize)> { } fn split_top_level(input: &str, delimiter: char) -> Vec { + split_top_level_multi(input, &[delimiter]) +} + +fn split_top_level_multi(input: &str, delimiters: &[char]) -> Vec { let mut state = ScanState::default(); let mut start = 0usize; let mut parts = Vec::new(); for (index, ch) in input.char_indices() { - if !state.in_string() && ch == delimiter && state.depth.is_top_level() { + if !state.in_string() && state.depth.is_top_level() && delimiters.contains(&ch) { let part = input[start..index].trim(); if !part.is_empty() { parts.push(part.to_string()); @@ -531,6 +546,19 @@ fn parse_property(input: &str) -> Option<(String, &str)> { Some((name, input[colon_index + 1..].trim_start())) } +fn strip_leading_block_comments(input: &str) -> &str { + let mut rest = input.trim_start(); + loop { + let Some(after_prefix) = rest.strip_prefix("/*") else { + return rest; + }; + let Some(end_rel) = after_prefix.find("*/") else { + return rest; + }; + rest = after_prefix[end_rel + 2..].trim_start(); + } +} + fn parse_property_name(input: &str) -> Option { let trimmed = input.trim_start(); if trimmed.is_empty() { @@ -1222,7 +1250,6 @@ fn generate_index_ts(out_dir: &Path) -> Result { #[cfg(test)] mod tests { use super::*; - use crate::protocol::common::EXPERIMENTAL_CLIENT_METHODS; use crate::protocol::v2; use anyhow::Result; use pretty_assertions::assert_eq; @@ -1257,10 +1284,10 @@ mod tests { generate_ts_with_options(&output_dir, None, options)?; let client_request_ts = fs::read_to_string(output_dir.join("ClientRequest.ts"))?; - assert_eq!(client_request_ts.contains("collaborationMode/list"), false); + assert_eq!(client_request_ts.contains("mock/experimentalMethod"), false); let thread_start_ts = fs::read_to_string(output_dir.join("v2").join("ThreadStartParams.ts"))?; - assert_eq!(thread_start_ts.contains("dynamicTools"), false); + assert_eq!(thread_start_ts.contains("mockExperimentalField"), false); let mut undefined_offenders = Vec::new(); let mut optional_nullable_offenders = BTreeSet::new(); @@ -1437,7 +1464,7 @@ mod tests { } #[test] - fn stable_schema_filter_removes_dynamic_tools() -> Result<()> { + fn stable_schema_filter_removes_mock_thread_start_field() -> Result<()> { let output_dir = std::env::temp_dir().join(format!("codex_schema_{}", Uuid::now_v7())); fs::create_dir(&output_dir)?; let schema = write_json_schema_with_return::( @@ -1457,13 +1484,45 @@ mod tests { let properties = def_schema["properties"] .as_object() .expect("ThreadStartParams should have properties"); - assert_eq!(properties.contains_key("dynamicTools"), false); + assert_eq!(properties.contains_key("mockExperimentalField"), false); let _cleanup = fs::remove_dir_all(&output_dir); Ok(()) } #[test] - fn stable_schema_filter_removes_experimental_methods() -> Result<()> { + fn thread_start_ts_filter_handles_interface_shape() -> Result<()> { + let output_dir = std::env::temp_dir().join(format!("codex_ts_filter_{}", Uuid::now_v7())); + let v2_dir = output_dir.join("v2"); + fs::create_dir_all(&v2_dir)?; + + struct TempDirGuard(PathBuf); + + impl Drop for TempDirGuard { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + let _guard = TempDirGuard(output_dir.clone()); + let path = v2_dir.join("ThreadStartParams.ts"); + let content = r#"export interface ThreadStartParams { + model: string | null; + mockExperimentalField: string | null; + experimentalRawEvents: boolean; +} +"#; + fs::write(&path, content)?; + + let registered_fields = experimental_fields(); + filter_thread_start_params_ts(&output_dir, ®istered_fields)?; + + let filtered = fs::read_to_string(&path)?; + assert_eq!(filtered.contains("mockExperimentalField"), false); + Ok(()) + } + + #[test] + fn stable_schema_filter_removes_mock_experimental_method() -> Result<()> { let output_dir = std::env::temp_dir().join(format!("codex_schema_{}", Uuid::now_v7())); fs::create_dir(&output_dir)?; let schema = @@ -1472,13 +1531,7 @@ mod tests { filter_experimental_schema(&mut bundle)?; let bundle_str = serde_json::to_string(&bundle)?; - for method in EXPERIMENTAL_CLIENT_METHODS - .iter() - .copied() - .filter(|method| !method.is_empty()) - { - assert_eq!(bundle_str.contains(method), false); - } + assert_eq!(bundle_str.contains("mock/experimentalMethod"), false); let _cleanup = fs::remove_dir_all(&output_dir); Ok(()) } @@ -1491,20 +1544,17 @@ mod tests { let thread_start_json = fs::read_to_string(output_dir.join("v2").join("ThreadStartParams.json"))?; - assert_eq!(thread_start_json.contains("dynamicTools"), false); + assert_eq!(thread_start_json.contains("mockExperimentalField"), false); let client_request_json = fs::read_to_string(output_dir.join("ClientRequest.json"))?; - for method in EXPERIMENTAL_CLIENT_METHODS - .iter() - .copied() - .filter(|method| !method.is_empty()) - { - assert_eq!(client_request_json.contains(method), false); - } + assert_eq!( + client_request_json.contains("mock/experimentalMethod"), + false + ); let bundle_json = fs::read_to_string(output_dir.join("codex_app_server_protocol.schemas.json"))?; - assert_eq!(bundle_json.contains("dynamicTools"), false); + assert_eq!(bundle_json.contains("mockExperimentalField"), false); let _cleanup = fs::remove_dir_all(&output_dir); Ok(()) diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 8eed7e0269..f41a47c954 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -236,6 +236,12 @@ client_request_definitions! { params: v2::CollaborationModeListParams, response: v2::CollaborationModeListResponse, }, + #[experimental("mock/experimentalMethod")] + /// EXPERIMENTAL - test-only method used to validate experimental gating. + MockExperimentalMethod => "mock/experimentalMethod" { + params: v2::MockExperimentalMethodParams, + response: v2::MockExperimentalMethodResponse, + }, McpServerOauthLogin => "mcpServer/oauth/login" { params: v2::McpServerOauthLoginParams, @@ -1047,29 +1053,25 @@ mod tests { } #[test] - fn collaboration_mode_list_is_marked_experimental() { - let request = ClientRequest::CollaborationModeList { + fn mock_experimental_method_is_marked_experimental() { + let request = ClientRequest::MockExperimentalMethod { request_id: RequestId::Integer(1), - params: v2::CollaborationModeListParams::default(), + params: v2::MockExperimentalMethodParams::default(), }; let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&request); - assert_eq!(reason, Some("collaborationMode/list")); + assert_eq!(reason, Some("mock/experimentalMethod")); } #[test] - fn thread_start_dynamic_tools_is_marked_experimental() { + fn thread_start_mock_field_is_marked_experimental() { let request = ClientRequest::ThreadStart { request_id: RequestId::Integer(1), params: v2::ThreadStartParams { - dynamic_tools: Some(vec![v2::DynamicToolSpec { - name: "tool".to_string(), - description: "desc".to_string(), - input_schema: json!({"type": "object"}), - }]), + mock_experimental_field: Some("mock".to_string()), ..Default::default() }, }; let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&request); - assert_eq!(reason, Some("thread/start.dynamicTools")); + assert_eq!(reason, Some("thread/start.mockExperimentalField")); } } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 0b6d0c5994..3ca344ee49 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1184,6 +1184,10 @@ pub struct ThreadStartParams { pub ephemeral: Option, #[experimental("thread/start.dynamicTools")] pub dynamic_tools: Option>, + /// Test-only experimental field used to validate experimental gating and + /// schema filtering behavior in a stable way. + #[experimental("thread/start.mockExperimentalField")] + pub mock_experimental_field: Option, /// If true, opt into emitting raw response items on the event stream. /// /// This is for internal use only (e.g. Codex Cloud). @@ -1192,6 +1196,22 @@ pub struct ThreadStartParams { pub experimental_raw_events: bool, } +#[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct MockExperimentalMethodParams { + /// Test-only payload field. + pub value: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct MockExperimentalMethodResponse { + /// Echoes the input `value`. + pub echoed: Option, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index aae369a19d..1fca0cd066 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -69,6 +69,8 @@ use codex_app_server_protocol::McpServerOauthLoginParams; use codex_app_server_protocol::McpServerOauthLoginResponse; use codex_app_server_protocol::McpServerRefreshResponse; use codex_app_server_protocol::McpServerStatus; +use codex_app_server_protocol::MockExperimentalMethodParams; +use codex_app_server_protocol::MockExperimentalMethodResponse; use codex_app_server_protocol::ModelListParams; use codex_app_server_protocol::ModelListResponse; use codex_app_server_protocol::NewConversationParams; @@ -504,6 +506,9 @@ impl CodexMessageProcessor { .await; }); } + ClientRequest::MockExperimentalMethod { request_id, params } => { + self.mock_experimental_method(request_id, params).await; + } ClientRequest::McpServerOauthLogin { request_id, params } => { self.mcp_server_oauth_login(request_id, params).await; } @@ -1603,6 +1608,7 @@ impl CodexMessageProcessor { base_instructions, developer_instructions, dynamic_tools, + mock_experimental_field: _mock_experimental_field, experimental_raw_events, personality, ephemeral, @@ -2976,6 +2982,16 @@ impl CodexMessageProcessor { outgoing.send_response(request_id, response).await; } + async fn mock_experimental_method( + &self, + request_id: RequestId, + params: MockExperimentalMethodParams, + ) { + let MockExperimentalMethodParams { value } = params; + let response = MockExperimentalMethodResponse { echoed: value }; + self.outgoing.send_response(request_id, response).await; + } + async fn mcp_server_refresh(&self, request_id: RequestId, _params: Option<()>) { let config = match self.load_latest_config().await { Ok(config) => config, diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index 171dd383e2..4804a8cd37 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -38,6 +38,7 @@ use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::ListConversationsParams; use codex_app_server_protocol::LoginAccountParams; use codex_app_server_protocol::LoginApiKeyParams; +use codex_app_server_protocol::MockExperimentalMethodParams; use codex_app_server_protocol::ModelListParams; use codex_app_server_protocol::NewConversationParams; use codex_app_server_protocol::RemoveConversationListenerParams; @@ -477,6 +478,15 @@ impl McpProcess { self.send_request("collaborationMode/list", params).await } + /// Send a `mock/experimentalMethod` JSON-RPC request. + pub async fn send_mock_experimental_method_request( + &mut self, + params: MockExperimentalMethodParams, + ) -> anyhow::Result { + let params = Some(serde_json::to_value(params)?); + self.send_request("mock/experimentalMethod", params).await + } + /// Send a `resumeConversation` JSON-RPC request. pub async fn send_resume_conversation_request( &mut self, diff --git a/codex-rs/app-server/tests/suite/v2/experimental_api.rs b/codex-rs/app-server/tests/suite/v2/experimental_api.rs index ac849458cb..5116633a48 100644 --- a/codex-rs/app-server/tests/suite/v2/experimental_api.rs +++ b/codex-rs/app-server/tests/suite/v2/experimental_api.rs @@ -4,17 +4,15 @@ use app_test_support::McpProcess; use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::to_response; use codex_app_server_protocol::ClientInfo; -use codex_app_server_protocol::CollaborationModeListParams; -use codex_app_server_protocol::DynamicToolSpec; use codex_app_server_protocol::InitializeCapabilities; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::MockExperimentalMethodParams; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use pretty_assertions::assert_eq; -use serde_json::json; use std::path::Path; use std::time::Duration; use tempfile::TempDir; @@ -23,7 +21,7 @@ use tokio::time::timeout; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); #[tokio::test] -async fn collaboration_mode_list_requires_experimental_api_capability() -> Result<()> { +async fn mock_experimental_method_requires_experimental_api_capability() -> Result<()> { let codex_home = TempDir::new()?; let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -40,19 +38,19 @@ async fn collaboration_mode_list_requires_experimental_api_capability() -> Resul }; let request_id = mcp - .send_list_collaboration_modes_request(CollaborationModeListParams {}) + .send_mock_experimental_method_request(MockExperimentalMethodParams::default()) .await?; let error = timeout( DEFAULT_TIMEOUT, mcp.read_stream_until_error_message(RequestId::Integer(request_id)), ) .await??; - assert_experimental_capability_error(error, "collaborationMode/list"); + assert_experimental_capability_error(error, "mock/experimentalMethod"); Ok(()) } #[tokio::test] -async fn thread_start_dynamic_tools_requires_experimental_api_capability() -> Result<()> { +async fn thread_start_mock_field_requires_experimental_api_capability() -> Result<()> { let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await; let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), &server.uri())?; @@ -70,21 +68,9 @@ async fn thread_start_dynamic_tools_requires_experimental_api_capability() -> Re anyhow::bail!("expected initialize response, got {init:?}"); }; - let dynamic_tool = DynamicToolSpec { - name: "demo_tool".to_string(), - description: "Demo dynamic tool".to_string(), - input_schema: json!({ - "type": "object", - "properties": { - "city": { "type": "string" } - }, - "required": ["city"], - "additionalProperties": false, - }), - }; let request_id = mcp .send_thread_start_request(ThreadStartParams { - dynamic_tools: Some(vec![dynamic_tool]), + mock_experimental_field: Some("mock".to_string()), ..Default::default() }) .await?; @@ -94,7 +80,7 @@ async fn thread_start_dynamic_tools_requires_experimental_api_capability() -> Re mcp.read_stream_until_error_message(RequestId::Integer(request_id)), ) .await??; - assert_experimental_capability_error(error, "thread/start.dynamicTools"); + assert_experimental_capability_error(error, "thread/start.mockExperimentalField"); Ok(()) }