mirror of
https://github.com/openai/codex.git
synced 2026-04-29 00:55:38 +00:00
## Summary
This change makes `AskForApproval::Reject` gate correctly anywhere it
appears inside otherwise-stable app-server protocol types.
Previously, experimental gating for `approval_policy: Reject` was
handled with request-specific logic in `ClientRequest` detection. That
covered a few request params types, but it did not generalize to other
nested uses such as `ProfileV2`, `Config`, `ConfigReadResponse`, or
`ConfigRequirements`.
This PR replaces that ad hoc handling with a generic nested experimental
propagation mechanism.
## Testing
seeing this when run app-server-test-client without experimental api
enabled:
```
initialize response: InitializeResponse { user_agent: "codex-toy-app-server/0.0.0 (Mac OS 26.3.1; arm64) vscode/2.4.36 (codex-toy-app-server; 0.0.0)" }
> {
> "id": "50244f6a-270a-425d-ace0-e9e98205bde7",
> "method": "thread/start",
> "params": {
> "approvalPolicy": {
> "reject": {
> "mcp_elicitations": false,
> "request_permissions": true,
> "rules": false,
> "sandbox_approval": true
> }
> },
> "baseInstructions": null,
> "config": null,
> "cwd": null,
> "developerInstructions": null,
> "dynamicTools": null,
> "ephemeral": null,
> "experimentalRawEvents": false,
> "mockExperimentalField": null,
> "model": null,
> "modelProvider": null,
> "persistExtendedHistory": false,
> "personality": null,
> "sandbox": null,
> "serviceName": null
> }
> }
< {
< "error": {
< "code": -32600,
< "message": "askForApproval.reject requires experimentalApi capability"
< },
< "id": "50244f6a-270a-425d-ace0-e9e98205bde7"
< }
[verified] thread/start rejected approvalPolicy=Reject without experimentalApi
```
---------
Co-authored-by: celia-oai <celia@openai.com>
241 lines
7.5 KiB
Rust
241 lines
7.5 KiB
Rust
use anyhow::Result;
|
|
use app_test_support::DEFAULT_CLIENT_NAME;
|
|
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::AskForApproval;
|
|
use codex_app_server_protocol::ClientInfo;
|
|
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::ThreadRealtimeStartParams;
|
|
use codex_app_server_protocol::ThreadStartParams;
|
|
use codex_app_server_protocol::ThreadStartResponse;
|
|
use pretty_assertions::assert_eq;
|
|
use std::path::Path;
|
|
use std::time::Duration;
|
|
use tempfile::TempDir;
|
|
use tokio::time::timeout;
|
|
|
|
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
|
|
|
|
#[tokio::test]
|
|
async fn mock_experimental_method_requires_experimental_api_capability() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
|
|
let init = mcp
|
|
.initialize_with_capabilities(
|
|
default_client_info(),
|
|
Some(InitializeCapabilities {
|
|
experimental_api: false,
|
|
opt_out_notification_methods: None,
|
|
}),
|
|
)
|
|
.await?;
|
|
let JSONRPCMessage::Response(_) = init else {
|
|
anyhow::bail!("expected initialize response, got {init:?}");
|
|
};
|
|
|
|
let request_id = mcp
|
|
.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, "mock/experimentalMethod");
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn realtime_conversation_start_requires_experimental_api_capability() -> Result<()> {
|
|
let codex_home = TempDir::new()?;
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
|
|
let init = mcp
|
|
.initialize_with_capabilities(
|
|
default_client_info(),
|
|
Some(InitializeCapabilities {
|
|
experimental_api: false,
|
|
opt_out_notification_methods: None,
|
|
}),
|
|
)
|
|
.await?;
|
|
let JSONRPCMessage::Response(_) = init else {
|
|
anyhow::bail!("expected initialize response, got {init:?}");
|
|
};
|
|
|
|
let request_id = mcp
|
|
.send_thread_realtime_start_request(ThreadRealtimeStartParams {
|
|
thread_id: "thr_123".to_string(),
|
|
prompt: "hello".to_string(),
|
|
session_id: None,
|
|
})
|
|
.await?;
|
|
let error = timeout(
|
|
DEFAULT_TIMEOUT,
|
|
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
assert_experimental_capability_error(error, "thread/realtime/start");
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
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())?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
let init = mcp
|
|
.initialize_with_capabilities(
|
|
default_client_info(),
|
|
Some(InitializeCapabilities {
|
|
experimental_api: false,
|
|
opt_out_notification_methods: None,
|
|
}),
|
|
)
|
|
.await?;
|
|
let JSONRPCMessage::Response(_) = init else {
|
|
anyhow::bail!("expected initialize response, got {init:?}");
|
|
};
|
|
|
|
let request_id = mcp
|
|
.send_thread_start_request(ThreadStartParams {
|
|
mock_experimental_field: Some("mock".to_string()),
|
|
..Default::default()
|
|
})
|
|
.await?;
|
|
|
|
let error = timeout(
|
|
DEFAULT_TIMEOUT,
|
|
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
assert_experimental_capability_error(error, "thread/start.mockExperimentalField");
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn thread_start_without_dynamic_tools_allows_without_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())?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
let init = mcp
|
|
.initialize_with_capabilities(
|
|
default_client_info(),
|
|
Some(InitializeCapabilities {
|
|
experimental_api: false,
|
|
opt_out_notification_methods: None,
|
|
}),
|
|
)
|
|
.await?;
|
|
let JSONRPCMessage::Response(_) = init else {
|
|
anyhow::bail!("expected initialize response, got {init:?}");
|
|
};
|
|
|
|
let request_id = mcp
|
|
.send_thread_start_request(ThreadStartParams {
|
|
model: Some("mock-model".to_string()),
|
|
..Default::default()
|
|
})
|
|
.await?;
|
|
let response: JSONRPCResponse = timeout(
|
|
DEFAULT_TIMEOUT,
|
|
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
let _: ThreadStartResponse = to_response(response)?;
|
|
Ok(())
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn thread_start_reject_approval_policy_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())?;
|
|
|
|
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
|
let init = mcp
|
|
.initialize_with_capabilities(
|
|
default_client_info(),
|
|
Some(InitializeCapabilities {
|
|
experimental_api: false,
|
|
opt_out_notification_methods: None,
|
|
}),
|
|
)
|
|
.await?;
|
|
let JSONRPCMessage::Response(_) = init else {
|
|
anyhow::bail!("expected initialize response, got {init:?}");
|
|
};
|
|
|
|
let request_id = mcp
|
|
.send_thread_start_request(ThreadStartParams {
|
|
approval_policy: Some(AskForApproval::Reject {
|
|
sandbox_approval: true,
|
|
rules: false,
|
|
request_permissions: true,
|
|
mcp_elicitations: false,
|
|
}),
|
|
..Default::default()
|
|
})
|
|
.await?;
|
|
|
|
let error = timeout(
|
|
DEFAULT_TIMEOUT,
|
|
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
|
|
)
|
|
.await??;
|
|
assert_experimental_capability_error(error, "askForApproval.reject");
|
|
Ok(())
|
|
}
|
|
|
|
fn default_client_info() -> ClientInfo {
|
|
ClientInfo {
|
|
name: DEFAULT_CLIENT_NAME.to_string(),
|
|
title: None,
|
|
version: "0.1.0".to_string(),
|
|
}
|
|
}
|
|
|
|
fn assert_experimental_capability_error(error: JSONRPCError, reason: &str) {
|
|
assert_eq!(error.error.code, -32600);
|
|
assert_eq!(
|
|
error.error.message,
|
|
format!("{reason} requires experimentalApi capability")
|
|
);
|
|
assert_eq!(error.error.data, None);
|
|
}
|
|
|
|
fn create_config_toml(codex_home: &Path, server_uri: &str) -> std::io::Result<()> {
|
|
let config_toml = codex_home.join("config.toml");
|
|
std::fs::write(
|
|
config_toml,
|
|
format!(
|
|
r#"
|
|
model = "mock-model"
|
|
approval_policy = "never"
|
|
sandbox_mode = "read-only"
|
|
|
|
model_provider = "mock_provider"
|
|
|
|
[model_providers.mock_provider]
|
|
name = "Mock provider for test"
|
|
base_url = "{server_uri}/v1"
|
|
wire_api = "responses"
|
|
request_max_retries = 0
|
|
stream_max_retries = 0
|
|
"#
|
|
),
|
|
)
|
|
}
|