mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[ez][codex-rs] Support apps._default.default_tools_approval_mode (#27965)
[from codex] ## Summary - add `default_tools_approval_mode` to `[apps._default]` and expose it through app-server v2 `config/read` - apply it after managed, per-tool, and per-app approval settings, before the built-in `auto` fallback - document the precedence, regenerate config/app-server schemas, and add unit plus end-to-end approval coverage ## Configuration ```toml [apps._default] default_tools_approval_mode = "prompt" ``` The effective precedence is managed requirements, tool-specific `approval_mode`, app-specific `default_tools_approval_mode`, `apps._default.default_tools_approval_mode`, then `auto`. ## Test plan - `just write-config-schema` - `just write-app-server-schema` - `just write-app-server-schema --experimental` - `just test -p codex-core app_tool_policy` - `just test -p codex-core mcp_turn_metadata` - `just test -p codex-config` - `just test -p codex-app-server-protocol` - `just test -p codex-app-server config_read_includes_apps` - `just fix -p codex-config -p codex-core -p codex-app-server-protocol -p codex-app-server` - `just fmt`
This commit is contained in:
committed by
GitHub
Unverified
parent
0318381762
commit
c78911e37f
+10
@@ -6696,6 +6696,16 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"default_tools_approval_mode": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/v2/AppToolApproval"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
]
|
||||
},
|
||||
"destructive_enabled": {
|
||||
"default": true,
|
||||
"type": "boolean"
|
||||
|
||||
+10
@@ -898,6 +898,16 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"default_tools_approval_mode": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AppToolApproval"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
]
|
||||
},
|
||||
"destructive_enabled": {
|
||||
"default": true,
|
||||
"type": "boolean"
|
||||
|
||||
@@ -143,6 +143,16 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"default_tools_approval_mode": {
|
||||
"anyOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AppToolApproval"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
]
|
||||
},
|
||||
"destructive_enabled": {
|
||||
"default": true,
|
||||
"type": "boolean"
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// 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 { AppToolApproval } from "./AppToolApproval";
|
||||
import type { ApprovalsReviewer } from "./ApprovalsReviewer";
|
||||
|
||||
export type AppsDefaultConfig = { enabled: boolean, approvals_reviewer: ApprovalsReviewer | null, destructive_enabled: boolean, open_world_enabled: boolean, };
|
||||
export type AppsDefaultConfig = { enabled: boolean, approvals_reviewer: ApprovalsReviewer | null, destructive_enabled: boolean, open_world_enabled: boolean, default_tools_approval_mode: AppToolApproval | null, };
|
||||
|
||||
@@ -177,6 +177,7 @@ pub struct AppsDefaultConfig {
|
||||
pub destructive_enabled: bool,
|
||||
#[serde(default = "default_enabled")]
|
||||
pub open_world_enabled: bool,
|
||||
pub default_tools_approval_mode: Option<AppToolApproval>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
|
||||
@@ -1825,15 +1825,24 @@ approvals_reviewer = "auto_review"
|
||||
|
||||
[apps._default]
|
||||
approvals_reviewer = "user"
|
||||
default_tools_approval_mode = "prompt"
|
||||
|
||||
[apps.demo-app]
|
||||
approvals_reviewer = "auto_review"
|
||||
default_tools_approval_mode = "approve"
|
||||
```
|
||||
|
||||
Setting the app value to `"user"` routes its approval prompts to the user
|
||||
instead of Guardian; setting it to `"auto_review"` opts that app into Guardian
|
||||
review when allowed by configuration requirements.
|
||||
|
||||
Use `apps._default.default_tools_approval_mode` to set the approval mode for
|
||||
tools without a per-app or per-tool override. Supported values are `"auto"`,
|
||||
`"prompt"`, and `"approve"`. Tool-level `approval_mode` takes precedence over
|
||||
the per-app `default_tools_approval_mode`, which takes precedence over the
|
||||
`apps._default` value. Managed tool requirements take precedence over all of
|
||||
these settings. When none are configured, the mode defaults to `"auto"`.
|
||||
|
||||
Invoke an app by inserting `$<app-slug>` in the text input. The slug is derived from the app name and lowercased with non-alphanumeric characters replaced by `-` (for example, "Demo App" becomes `$demo-app`). Add a `mention` input item (recommended) so the server uses the exact `app://<connector-id>` path rather than guessing by name. Plugins use the same `mention` item shape, but with `plugin://<plugin-name>@<marketplace-name>` paths from `plugin/installed` or `plugin/list`.
|
||||
|
||||
Example:
|
||||
|
||||
@@ -363,6 +363,7 @@ async fn config_read_includes_apps() -> Result<()> {
|
||||
r#"
|
||||
[apps._default]
|
||||
approvals_reviewer = "auto_review"
|
||||
default_tools_approval_mode = "approve"
|
||||
|
||||
[apps.app1]
|
||||
enabled = false
|
||||
@@ -402,6 +403,7 @@ default_tools_approval_mode = "prompt"
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
destructive_enabled: true,
|
||||
open_world_enabled: true,
|
||||
default_tools_approval_mode: Some(AppToolApproval::Approve),
|
||||
}),
|
||||
apps: std::collections::HashMap::from([(
|
||||
"app1".to_string(),
|
||||
@@ -427,6 +429,16 @@ default_tools_approval_mode = "prompt"
|
||||
profile: None,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
origins
|
||||
.get("apps._default.default_tools_approval_mode")
|
||||
.expect("origin")
|
||||
.name,
|
||||
ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
profile: None,
|
||||
}
|
||||
);
|
||||
assert_eq!(
|
||||
origins.get("apps.app1.enabled").expect("origin").name,
|
||||
ConfigLayerSource::User {
|
||||
|
||||
@@ -417,6 +417,10 @@ pub struct AppsDefaultConfig {
|
||||
skip_serializing_if = "std::clone::Clone::clone"
|
||||
)]
|
||||
pub open_world_enabled: bool,
|
||||
|
||||
/// Approval mode for tools unless overridden by per-app or per-tool settings.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub default_tools_approval_mode: Option<AppToolApproval>,
|
||||
}
|
||||
|
||||
/// Per-tool settings for a single app tool.
|
||||
|
||||
@@ -162,6 +162,12 @@ fn app_tool_policy_from_apps_config(
|
||||
let approval = managed_approval
|
||||
.or_else(|| tool_config.and_then(|tool| tool.approval_mode))
|
||||
.or_else(|| app.and_then(|app| app.default_tools_approval_mode))
|
||||
.or_else(|| {
|
||||
input
|
||||
.connector_id
|
||||
.and(apps_config.default.as_ref())
|
||||
.and_then(|defaults| defaults.default_tools_approval_mode)
|
||||
})
|
||||
.unwrap_or(AppToolApproval::Auto);
|
||||
|
||||
if !app_is_enabled(apps_config, input.connector_id) {
|
||||
|
||||
@@ -521,9 +521,59 @@ fn default_tools_enable_overrides_app_level_hints() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_uses_default_tools_approval_mode() {
|
||||
fn evaluator_uses_apps_default_tools_approval_mode_only_with_connector_id() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: None,
|
||||
default: Some(AppsDefaultConfig {
|
||||
default_tools_approval_mode: Some(AppToolApproval::Prompt),
|
||||
..defaults(
|
||||
/*enabled*/ true, /*destructive_enabled*/ true,
|
||||
/*open_world_enabled*/ true,
|
||||
)
|
||||
}),
|
||||
apps: HashMap::new(),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
[
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
Some("calendar"),
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
policy_from_apps_config(
|
||||
Some(&apps_config),
|
||||
/*connector_id*/ None,
|
||||
"events/list",
|
||||
/*tool_title*/ None,
|
||||
/*destructive_hint*/ None,
|
||||
/*open_world_hint*/ None,
|
||||
/*managed_approval*/ None,
|
||||
),
|
||||
],
|
||||
[
|
||||
AppToolPolicy {
|
||||
enabled: true,
|
||||
approval: AppToolApproval::Prompt,
|
||||
},
|
||||
AppToolPolicy::default(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluator_prefers_app_default_tools_approval_mode_over_apps_default() {
|
||||
let apps_config = AppsConfigToml {
|
||||
default: Some(AppsDefaultConfig {
|
||||
default_tools_approval_mode: Some(AppToolApproval::Approve),
|
||||
..defaults(
|
||||
/*enabled*/ true, /*destructive_enabled*/ true,
|
||||
/*open_world_enabled*/ true,
|
||||
)
|
||||
}),
|
||||
apps: HashMap::from([(
|
||||
"calendar".to_string(),
|
||||
AppConfig {
|
||||
@@ -720,5 +770,6 @@ fn defaults(
|
||||
approvals_reviewer: None,
|
||||
destructive_enabled,
|
||||
open_world_enabled,
|
||||
default_tools_approval_mode: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -218,6 +218,14 @@
|
||||
],
|
||||
"description": "Reviewer for approval prompts unless overridden by per-app settings."
|
||||
},
|
||||
"default_tools_approval_mode": {
|
||||
"allOf": [
|
||||
{
|
||||
"$ref": "#/definitions/AppToolApproval"
|
||||
}
|
||||
],
|
||||
"description": "Approval mode for tools unless overridden by per-app or per-tool settings."
|
||||
},
|
||||
"destructive_enabled": {
|
||||
"description": "Whether tools with `destructive_hint = true` are allowed by default.",
|
||||
"type": "boolean"
|
||||
|
||||
@@ -60,7 +60,7 @@ default_tools_approval_mode = "{approval_mode}"
|
||||
.with_user_config(&user_config_path, user_config);
|
||||
}
|
||||
|
||||
fn set_calendar_approval_mode_and_default_reviewer(
|
||||
fn set_default_app_approval_mode_and_reviewer(
|
||||
config: &mut Config,
|
||||
approval_mode: AppToolApproval,
|
||||
default_approvals_reviewer: ApprovalsReviewer,
|
||||
@@ -75,8 +75,6 @@ fn set_calendar_approval_mode_and_default_reviewer(
|
||||
r#"
|
||||
[apps._default]
|
||||
approvals_reviewer = "{default_approvals_reviewer}"
|
||||
|
||||
[apps.calendar]
|
||||
default_tools_approval_mode = "{approval_mode}"
|
||||
"#
|
||||
))
|
||||
@@ -167,7 +165,7 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R
|
||||
.features
|
||||
.enable(Feature::ToolCallMcpElicitation)
|
||||
.expect("test config should allow feature update");
|
||||
set_calendar_approval_mode_and_default_reviewer(
|
||||
set_default_app_approval_mode_and_reviewer(
|
||||
config,
|
||||
AppToolApproval::Prompt,
|
||||
ApprovalsReviewer::User,
|
||||
@@ -231,7 +229,8 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request() -> R
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apps_default_auto_review_routes_actual_mcp_approval_to_guardian() -> Result<()> {
|
||||
async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guardian() -> Result<()>
|
||||
{
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
@@ -285,7 +284,7 @@ async fn apps_default_auto_review_routes_actual_mcp_approval_to_guardian() -> Re
|
||||
.features
|
||||
.enable(Feature::ToolCallMcpElicitation)
|
||||
.expect("test config should allow feature update");
|
||||
set_calendar_approval_mode_and_default_reviewer(
|
||||
set_default_app_approval_mode_and_reviewer(
|
||||
config,
|
||||
AppToolApproval::Prompt,
|
||||
ApprovalsReviewer::AutoReview,
|
||||
|
||||
Reference in New Issue
Block a user