app-server: propagate nested experimental gating for AskForApproval::Reject (#14191)
## 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>
This commit is contained in:
parent
722e8f08e1
commit
d5694529ca
6 changed files with 474 additions and 17 deletions
|
|
@ -1,3 +1,6 @@
|
|||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
|
||||
/// Marker trait for protocol types that can signal experimental usage.
|
||||
pub trait ExperimentalApi {
|
||||
/// Returns a short reason identifier when an experimental method or field is
|
||||
|
|
@ -28,8 +31,34 @@ pub fn experimental_required_message(reason: &str) -> String {
|
|||
format!("{reason} requires experimentalApi capability")
|
||||
}
|
||||
|
||||
impl<T: ExperimentalApi> ExperimentalApi for Option<T> {
|
||||
fn experimental_reason(&self) -> Option<&'static str> {
|
||||
self.as_ref().and_then(ExperimentalApi::experimental_reason)
|
||||
}
|
||||
}
|
||||
|
||||
impl<T: ExperimentalApi> ExperimentalApi for Vec<T> {
|
||||
fn experimental_reason(&self) -> Option<&'static str> {
|
||||
self.iter().find_map(ExperimentalApi::experimental_reason)
|
||||
}
|
||||
}
|
||||
|
||||
impl<K, V: ExperimentalApi, S> ExperimentalApi for HashMap<K, V, S> {
|
||||
fn experimental_reason(&self) -> Option<&'static str> {
|
||||
self.values().find_map(ExperimentalApi::experimental_reason)
|
||||
}
|
||||
}
|
||||
|
||||
impl<K: Ord, V: ExperimentalApi> ExperimentalApi for BTreeMap<K, V> {
|
||||
fn experimental_reason(&self) -> Option<&'static str> {
|
||||
self.values().find_map(ExperimentalApi::experimental_reason)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
|
||||
use super::ExperimentalApi as ExperimentalApiTrait;
|
||||
use codex_experimental_api_macros::ExperimentalApi;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
|
@ -48,6 +77,27 @@ mod tests {
|
|||
StableTuple(u8),
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(ExperimentalApi)]
|
||||
struct NestedFieldShape {
|
||||
#[experimental(nested)]
|
||||
inner: Option<EnumVariantShapes>,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(ExperimentalApi)]
|
||||
struct NestedCollectionShape {
|
||||
#[experimental(nested)]
|
||||
inners: Vec<EnumVariantShapes>,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(ExperimentalApi)]
|
||||
struct NestedMapShape {
|
||||
#[experimental(nested)]
|
||||
inners: HashMap<String, EnumVariantShapes>,
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_supports_all_enum_variant_shapes() {
|
||||
assert_eq!(
|
||||
|
|
@ -67,4 +117,56 @@ mod tests {
|
|||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_supports_nested_experimental_fields() {
|
||||
assert_eq!(
|
||||
ExperimentalApiTrait::experimental_reason(&NestedFieldShape {
|
||||
inner: Some(EnumVariantShapes::Named { value: 1 }),
|
||||
}),
|
||||
Some("enum/named")
|
||||
);
|
||||
assert_eq!(
|
||||
ExperimentalApiTrait::experimental_reason(&NestedFieldShape { inner: None }),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_supports_nested_collections() {
|
||||
assert_eq!(
|
||||
ExperimentalApiTrait::experimental_reason(&NestedCollectionShape {
|
||||
inners: vec![
|
||||
EnumVariantShapes::StableTuple(1),
|
||||
EnumVariantShapes::Tuple(2)
|
||||
],
|
||||
}),
|
||||
Some("enum/tuple")
|
||||
);
|
||||
assert_eq!(
|
||||
ExperimentalApiTrait::experimental_reason(&NestedCollectionShape {
|
||||
inners: Vec::new()
|
||||
}),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn derive_supports_nested_maps() {
|
||||
assert_eq!(
|
||||
ExperimentalApiTrait::experimental_reason(&NestedMapShape {
|
||||
inners: HashMap::from([(
|
||||
"default".to_string(),
|
||||
EnumVariantShapes::Named { value: 1 },
|
||||
)]),
|
||||
}),
|
||||
Some("enum/named")
|
||||
);
|
||||
assert_eq!(
|
||||
ExperimentalApiTrait::experimental_reason(&NestedMapShape {
|
||||
inners: HashMap::new(),
|
||||
}),
|
||||
None
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -44,15 +44,15 @@ pub enum AuthMode {
|
|||
|
||||
macro_rules! experimental_reason_expr {
|
||||
// If a request variant is explicitly marked experimental, that reason wins.
|
||||
(#[experimental($reason:expr)] $params:ident $(, $inspect_params:tt)?) => {
|
||||
(variant $variant:ident, #[experimental($reason:expr)] $params:ident $(, $inspect_params:tt)?) => {
|
||||
Some($reason)
|
||||
};
|
||||
// `inspect_params: true` is used when a method is mostly stable but needs
|
||||
// field-level gating from its params type (for example, ThreadStart).
|
||||
($params:ident, true) => {
|
||||
(variant $variant:ident, $params:ident, true) => {
|
||||
crate::experimental_api::ExperimentalApi::experimental_reason($params)
|
||||
};
|
||||
($params:ident $(, $inspect_params:tt)?) => {
|
||||
(variant $variant:ident, $params:ident $(, $inspect_params:tt)?) => {
|
||||
None
|
||||
};
|
||||
}
|
||||
|
|
@ -136,6 +136,7 @@ macro_rules! client_request_definitions {
|
|||
$(
|
||||
Self::$variant { params: _params, .. } => {
|
||||
experimental_reason_expr!(
|
||||
variant $variant,
|
||||
$(#[experimental($reason)])?
|
||||
_params
|
||||
$(, $inspect_params)?
|
||||
|
|
|
|||
|
|
@ -189,7 +189,9 @@ impl From<CoreCodexErrorInfo> for CodexErrorInfo {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[derive(
|
||||
Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS, ExperimentalApi,
|
||||
)]
|
||||
#[serde(rename_all = "kebab-case")]
|
||||
#[ts(rename_all = "kebab-case", export_to = "v2/")]
|
||||
pub enum AskForApproval {
|
||||
|
|
@ -198,6 +200,7 @@ pub enum AskForApproval {
|
|||
UnlessTrusted,
|
||||
OnFailure,
|
||||
OnRequest,
|
||||
#[experimental("askForApproval.reject")]
|
||||
Reject {
|
||||
sandbox_approval: bool,
|
||||
rules: bool,
|
||||
|
|
@ -502,12 +505,13 @@ pub struct DynamicToolSpec {
|
|||
pub input_schema: JsonValue,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ProfileV2 {
|
||||
pub model: Option<String>,
|
||||
pub model_provider: Option<String>,
|
||||
#[experimental(nested)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
pub service_tier: Option<ServiceTier>,
|
||||
pub model_reasoning_effort: Option<ReasoningEffort>,
|
||||
|
|
@ -606,6 +610,7 @@ pub struct Config {
|
|||
pub model_context_window: Option<i64>,
|
||||
pub model_auto_compact_token_limit: Option<i64>,
|
||||
pub model_provider: Option<String>,
|
||||
#[experimental(nested)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
pub sandbox_mode: Option<SandboxMode>,
|
||||
pub sandbox_workspace_write: Option<SandboxWorkspaceWrite>,
|
||||
|
|
@ -614,6 +619,7 @@ pub struct Config {
|
|||
pub web_search: Option<WebSearchMode>,
|
||||
pub tools: Option<ToolsV2>,
|
||||
pub profile: Option<String>,
|
||||
#[experimental(nested)]
|
||||
#[serde(default)]
|
||||
pub profiles: HashMap<String, ProfileV2>,
|
||||
pub instructions: Option<String>,
|
||||
|
|
@ -711,10 +717,11 @@ pub struct ConfigReadParams {
|
|||
pub cwd: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ConfigReadResponse {
|
||||
#[experimental(nested)]
|
||||
pub config: Config,
|
||||
pub origins: HashMap<String, ConfigLayerMetadata>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
|
@ -725,6 +732,7 @@ pub struct ConfigReadResponse {
|
|||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ConfigRequirements {
|
||||
#[experimental(nested)]
|
||||
pub allowed_approval_policies: Option<Vec<AskForApproval>>,
|
||||
pub allowed_sandbox_modes: Option<Vec<SandboxMode>>,
|
||||
pub allowed_web_search_modes: Option<Vec<WebSearchMode>>,
|
||||
|
|
@ -757,11 +765,12 @@ pub enum ResidencyRequirement {
|
|||
Us,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ConfigRequirementsReadResponse {
|
||||
/// Null if no requirements are configured (e.g. no requirements.toml/MDM entries).
|
||||
#[experimental(nested)]
|
||||
pub requirements: Option<ConfigRequirements>,
|
||||
}
|
||||
|
||||
|
|
@ -2229,6 +2238,7 @@ pub struct ThreadStartParams {
|
|||
pub service_tier: Option<Option<ServiceTier>>,
|
||||
#[ts(optional = nullable)]
|
||||
pub cwd: Option<String>,
|
||||
#[experimental(nested)]
|
||||
#[ts(optional = nullable)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
#[ts(optional = nullable)]
|
||||
|
|
@ -2282,7 +2292,7 @@ pub struct MockExperimentalMethodResponse {
|
|||
pub echoed: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ThreadStartResponse {
|
||||
|
|
@ -2291,6 +2301,7 @@ pub struct ThreadStartResponse {
|
|||
pub model_provider: String,
|
||||
pub service_tier: Option<ServiceTier>,
|
||||
pub cwd: PathBuf,
|
||||
#[experimental(nested)]
|
||||
pub approval_policy: AskForApproval,
|
||||
pub sandbox: SandboxPolicy,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
|
|
@ -2341,6 +2352,7 @@ pub struct ThreadResumeParams {
|
|||
pub service_tier: Option<Option<ServiceTier>>,
|
||||
#[ts(optional = nullable)]
|
||||
pub cwd: Option<String>,
|
||||
#[experimental(nested)]
|
||||
#[ts(optional = nullable)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
#[ts(optional = nullable)]
|
||||
|
|
@ -2360,7 +2372,7 @@ pub struct ThreadResumeParams {
|
|||
pub persist_extended_history: bool,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ThreadResumeResponse {
|
||||
|
|
@ -2369,6 +2381,7 @@ pub struct ThreadResumeResponse {
|
|||
pub model_provider: String,
|
||||
pub service_tier: Option<ServiceTier>,
|
||||
pub cwd: PathBuf,
|
||||
#[experimental(nested)]
|
||||
pub approval_policy: AskForApproval,
|
||||
pub sandbox: SandboxPolicy,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
|
|
@ -2410,6 +2423,7 @@ pub struct ThreadForkParams {
|
|||
pub service_tier: Option<Option<ServiceTier>>,
|
||||
#[ts(optional = nullable)]
|
||||
pub cwd: Option<String>,
|
||||
#[experimental(nested)]
|
||||
#[ts(optional = nullable)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
#[ts(optional = nullable)]
|
||||
|
|
@ -2427,7 +2441,7 @@ pub struct ThreadForkParams {
|
|||
pub persist_extended_history: bool,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ThreadForkResponse {
|
||||
|
|
@ -2436,6 +2450,7 @@ pub struct ThreadForkResponse {
|
|||
pub model_provider: String,
|
||||
pub service_tier: Option<ServiceTier>,
|
||||
pub cwd: PathBuf,
|
||||
#[experimental(nested)]
|
||||
pub approval_policy: AskForApproval,
|
||||
pub sandbox: SandboxPolicy,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
|
|
@ -3490,6 +3505,7 @@ pub struct TurnStartParams {
|
|||
#[ts(optional = nullable)]
|
||||
pub cwd: Option<PathBuf>,
|
||||
/// Override the approval policy for this turn and subsequent turns.
|
||||
#[experimental(nested)]
|
||||
#[ts(optional = nullable)]
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
/// Override the sandbox policy for this turn and subsequent turns.
|
||||
|
|
@ -6046,6 +6062,243 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ask_for_approval_reject_is_marked_experimental() {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(
|
||||
&AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
assert_eq!(
|
||||
crate::experimental_api::ExperimentalApi::experimental_reason(
|
||||
&AskForApproval::OnRequest,
|
||||
),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn profile_v2_reject_approval_policy_is_marked_experimental() {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&ProfileV2 {
|
||||
model: None,
|
||||
model_provider: None,
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
}),
|
||||
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.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_reject_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_provider: None,
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
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::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,
|
||||
additional: HashMap::new(),
|
||||
});
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_nested_profile_reject_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_provider: None,
|
||||
approval_policy: 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::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
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,
|
||||
additional: HashMap::new(),
|
||||
});
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_requirements_reject_allowed_approval_policy_is_marked_experimental() {
|
||||
let reason =
|
||||
crate::experimental_api::ExperimentalApi::experimental_reason(&ConfigRequirements {
|
||||
allowed_approval_policies: Some(vec![AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: true,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: false,
|
||||
}]),
|
||||
allowed_sandbox_modes: None,
|
||||
allowed_web_search_modes: None,
|
||||
feature_requirements: None,
|
||||
enforce_residency: None,
|
||||
network: None,
|
||||
});
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_request_thread_start_reject_approval_policy_is_marked_experimental() {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(
|
||||
&crate::ClientRequest::ThreadStart {
|
||||
request_id: crate::RequestId::Integer(1),
|
||||
params: ThreadStartParams {
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
request_permissions: true,
|
||||
mcp_elicitations: false,
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_request_thread_resume_reject_approval_policy_is_marked_experimental() {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(
|
||||
&crate::ClientRequest::ThreadResume {
|
||||
request_id: crate::RequestId::Integer(2),
|
||||
params: ThreadResumeParams {
|
||||
thread_id: "thr_123".to_string(),
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_request_thread_fork_reject_approval_policy_is_marked_experimental() {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(
|
||||
&crate::ClientRequest::ThreadFork {
|
||||
request_id: crate::RequestId::Integer(3),
|
||||
params: ThreadForkParams {
|
||||
thread_id: "thr_456".to_string(),
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: true,
|
||||
rules: false,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_request_turn_start_reject_approval_policy_is_marked_experimental() {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(
|
||||
&crate::ClientRequest::TurnStart {
|
||||
request_id: crate::RequestId::Integer(4),
|
||||
params: TurnStartParams {
|
||||
thread_id: "thr_123".to_string(),
|
||||
input: Vec::new(),
|
||||
approval_policy: Some(AskForApproval::Reject {
|
||||
sandbox_approval: false,
|
||||
rules: true,
|
||||
request_permissions: false,
|
||||
mcp_elicitations: true,
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(reason, Some("askForApproval.reject"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_server_elicitation_response_round_trips_rmcp_result() {
|
||||
let rmcp_result = rmcp::model::CreateElicitationResult {
|
||||
|
|
|
|||
|
|
@ -1319,6 +1319,7 @@ Examples of descriptor strings:
|
|||
|
||||
- `mock/experimentalMethod` (method-level gate)
|
||||
- `thread/start.mockExperimentalField` (field-level gate)
|
||||
- `askForApproval.reject` (enum-variant gate, for `approvalPolicy: { "reject": ... }`)
|
||||
|
||||
### For maintainers: Adding experimental fields and methods
|
||||
|
||||
|
|
@ -1335,6 +1336,28 @@ At runtime, clients must send `initialize` with `capabilities.experimentalApi =
|
|||
|
||||
3. In `app-server-protocol/src/protocol/common.rs`, keep the method stable and use `inspect_params: true` when only some fields are experimental (like `thread/start`). If the entire method is experimental, annotate the method variant with `#[experimental("method/name")]`.
|
||||
|
||||
Enum variants can be gated too:
|
||||
|
||||
```rust
|
||||
#[derive(ExperimentalApi)]
|
||||
enum AskForApproval {
|
||||
#[experimental("askForApproval.reject")]
|
||||
Reject { /* ... */ },
|
||||
}
|
||||
```
|
||||
|
||||
If a stable field contains a nested type that may itself be experimental, mark
|
||||
the field with `#[experimental(nested)]` so `ExperimentalApi` bubbles the nested
|
||||
reason up through the containing type:
|
||||
|
||||
```rust
|
||||
#[derive(ExperimentalApi)]
|
||||
struct ProfileV2 {
|
||||
#[experimental(nested)]
|
||||
approval_policy: Option<AskForApproval>,
|
||||
}
|
||||
```
|
||||
|
||||
For server-initiated request payloads, annotate the field the same way so schema generation treats it as experimental, and make sure app-server omits that field when the client did not opt into `experimentalApi`.
|
||||
|
||||
4. Regenerate protocol fixtures:
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ 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;
|
||||
|
|
@ -157,6 +158,47 @@ async fn thread_start_without_dynamic_tools_allows_without_experimental_api_capa
|
|||
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(),
|
||||
|
|
|
|||
|
|
@ -37,8 +37,7 @@ fn derive_for_struct(input: &DeriveInput, data: &DataStruct) -> TokenStream {
|
|||
let mut experimental_fields = Vec::new();
|
||||
let mut registrations = Vec::new();
|
||||
for field in &named.named {
|
||||
let reason = experimental_reason(&field.attrs);
|
||||
if let Some(reason) = reason {
|
||||
if let Some(reason) = experimental_reason(&field.attrs) {
|
||||
let expr = experimental_presence_expr(field, false);
|
||||
checks.push(quote! {
|
||||
if #expr {
|
||||
|
|
@ -65,6 +64,17 @@ fn derive_for_struct(input: &DeriveInput, data: &DataStruct) -> TokenStream {
|
|||
}
|
||||
});
|
||||
}
|
||||
} else if has_nested_experimental(field) {
|
||||
let Some(ident) = field.ident.as_ref() else {
|
||||
continue;
|
||||
};
|
||||
checks.push(quote! {
|
||||
if let Some(reason) =
|
||||
crate::experimental_api::ExperimentalApi::experimental_reason(&self.#ident)
|
||||
{
|
||||
return Some(reason);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
(checks, experimental_fields, registrations)
|
||||
|
|
@ -74,8 +84,7 @@ fn derive_for_struct(input: &DeriveInput, data: &DataStruct) -> TokenStream {
|
|||
let mut experimental_fields = Vec::new();
|
||||
let mut registrations = Vec::new();
|
||||
for (index, field) in unnamed.unnamed.iter().enumerate() {
|
||||
let reason = experimental_reason(&field.attrs);
|
||||
if let Some(reason) = reason {
|
||||
if let Some(reason) = experimental_reason(&field.attrs) {
|
||||
let expr = index_presence_expr(index, &field.ty);
|
||||
checks.push(quote! {
|
||||
if #expr {
|
||||
|
|
@ -100,6 +109,15 @@ fn derive_for_struct(input: &DeriveInput, data: &DataStruct) -> TokenStream {
|
|||
}
|
||||
}
|
||||
});
|
||||
} else if has_nested_experimental(field) {
|
||||
let index = syn::Index::from(index);
|
||||
checks.push(quote! {
|
||||
if let Some(reason) =
|
||||
crate::experimental_api::ExperimentalApi::experimental_reason(&self.#index)
|
||||
{
|
||||
return Some(reason);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
(checks, experimental_fields, registrations)
|
||||
|
|
@ -175,12 +193,30 @@ fn derive_for_enum(input: &DeriveInput, data: &DataEnum) -> TokenStream {
|
|||
}
|
||||
|
||||
fn experimental_reason(attrs: &[Attribute]) -> Option<LitStr> {
|
||||
let attr = attrs
|
||||
.iter()
|
||||
.find(|attr| attr.path().is_ident("experimental"))?;
|
||||
attrs.iter().find_map(experimental_reason_attr)
|
||||
}
|
||||
|
||||
fn experimental_reason_attr(attr: &Attribute) -> Option<LitStr> {
|
||||
if !attr.path().is_ident("experimental") {
|
||||
return None;
|
||||
}
|
||||
|
||||
attr.parse_args::<LitStr>().ok()
|
||||
}
|
||||
|
||||
fn has_nested_experimental(field: &Field) -> bool {
|
||||
field.attrs.iter().any(experimental_nested_attr)
|
||||
}
|
||||
|
||||
fn experimental_nested_attr(attr: &Attribute) -> bool {
|
||||
if !attr.path().is_ident("experimental") {
|
||||
return false;
|
||||
}
|
||||
|
||||
attr.parse_args::<Ident>()
|
||||
.is_ok_and(|ident| ident == "nested")
|
||||
}
|
||||
|
||||
fn field_serialized_name(field: &Field) -> Option<String> {
|
||||
let ident = field.ident.as_ref()?;
|
||||
let name = ident.to_string();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue