mirror of
https://github.com/openai/codex.git
synced 2026-05-08 21:32:33 +00:00
Compare commits
5 Commits
dev/mzeng/
...
pr19089
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4f63342d72 | ||
|
|
951be1a8a1 | ||
|
|
fb6308cf64 | ||
|
|
7730fb3ab8 | ||
|
|
08b5e96678 |
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -1859,7 +1859,6 @@ dependencies = [
|
||||
"codex-model-provider-info",
|
||||
"codex-models-manager",
|
||||
"codex-otel",
|
||||
"codex-plugin",
|
||||
"codex-protocol",
|
||||
"codex-rmcp-client",
|
||||
"codex-rollout",
|
||||
|
||||
@@ -40,7 +40,6 @@ codex-exec-server = { workspace = true }
|
||||
codex-features = { workspace = true }
|
||||
codex-git-utils = { workspace = true }
|
||||
codex-otel = { workspace = true }
|
||||
codex-plugin = { workspace = true }
|
||||
codex-shell-command = { workspace = true }
|
||||
codex-utils-cli = { workspace = true }
|
||||
codex-utils-pty = { workspace = true }
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use super::*;
|
||||
use codex_plugin::validate_plugin_segment;
|
||||
|
||||
impl CodexMessageProcessor {
|
||||
pub(super) async fn plugin_list(
|
||||
@@ -289,20 +288,24 @@ impl CodexMessageProcessor {
|
||||
let remote_plugin_service_config = RemotePluginServiceConfig {
|
||||
chatgpt_base_url: config.chatgpt_base_url.clone(),
|
||||
};
|
||||
if let Err(err) = validate_plugin_segment(&plugin_name, "plugin name") {
|
||||
if plugin_name.is_empty()
|
||||
|| !plugin_name
|
||||
.chars()
|
||||
.all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' || ch == '~')
|
||||
{
|
||||
self.send_invalid_request_error(
|
||||
request_id,
|
||||
format!("invalid remote plugin id: {err}"),
|
||||
"invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed"
|
||||
.to_string(),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
let remote_plugin_id = format!("{plugin_name}@{remote_marketplace_name}");
|
||||
let remote_detail = match codex_core_plugins::remote::fetch_remote_plugin_detail(
|
||||
&remote_plugin_service_config,
|
||||
auth.as_ref(),
|
||||
&remote_marketplace_name,
|
||||
&remote_plugin_id,
|
||||
&plugin_name,
|
||||
)
|
||||
.await
|
||||
{
|
||||
|
||||
@@ -963,7 +963,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() -
|
||||
let global_directory_body = r#"{
|
||||
"plugins": [
|
||||
{
|
||||
"id": "linear@chatgpt-global",
|
||||
"id": "plugins~Plugin_linear",
|
||||
"name": "linear",
|
||||
"scope": "GLOBAL",
|
||||
"installation_policy": "AVAILABLE",
|
||||
@@ -997,7 +997,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() -
|
||||
let global_installed_body = r#"{
|
||||
"plugins": [
|
||||
{
|
||||
"id": "linear@chatgpt-global",
|
||||
"id": "plugins~Plugin_linear",
|
||||
"name": "linear",
|
||||
"scope": "GLOBAL",
|
||||
"installation_policy": "AVAILABLE",
|
||||
@@ -1027,6 +1027,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() -
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/ps/plugins/list"))
|
||||
.and(query_param("scope", "GLOBAL"))
|
||||
.and(query_param("limit", "200"))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_string(global_directory_body))
|
||||
@@ -1035,6 +1036,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() -
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/ps/plugins/list"))
|
||||
.and(query_param("scope", "WORKSPACE"))
|
||||
.and(query_param("limit", "200"))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_string(empty_page_body))
|
||||
@@ -1085,7 +1087,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() -
|
||||
Some("ChatGPT Plugins")
|
||||
);
|
||||
assert_eq!(remote_marketplace.plugins.len(), 1);
|
||||
assert_eq!(remote_marketplace.plugins[0].id, "linear@chatgpt-global");
|
||||
assert_eq!(remote_marketplace.plugins[0].id, "plugins~Plugin_linear");
|
||||
assert_eq!(remote_marketplace.plugins[0].name, "linear");
|
||||
assert_eq!(remote_marketplace.plugins[0].source, PluginSource::Remote);
|
||||
assert_eq!(remote_marketplace.plugins[0].installed, true);
|
||||
@@ -1144,7 +1146,7 @@ async fn plugin_list_remote_marketplace_replaces_local_marketplace_with_same_nam
|
||||
let global_directory_body = r#"{
|
||||
"plugins": [
|
||||
{
|
||||
"id": "linear@chatgpt-global",
|
||||
"id": "plugins~Plugin_linear",
|
||||
"name": "linear",
|
||||
"scope": "GLOBAL",
|
||||
"installation_policy": "AVAILABLE",
|
||||
@@ -1170,33 +1172,30 @@ async fn plugin_list_remote_marketplace_replaces_local_marketplace_with_same_nam
|
||||
"next_page_token": null
|
||||
}
|
||||
}"#;
|
||||
for (path_suffix, scope, body) in [
|
||||
(
|
||||
"/backend-api/ps/plugins/list",
|
||||
"GLOBAL",
|
||||
global_directory_body,
|
||||
),
|
||||
("/backend-api/ps/plugins/list", "WORKSPACE", empty_page_body),
|
||||
(
|
||||
"/backend-api/ps/plugins/installed",
|
||||
"GLOBAL",
|
||||
empty_page_body,
|
||||
),
|
||||
(
|
||||
"/backend-api/ps/plugins/installed",
|
||||
"WORKSPACE",
|
||||
empty_page_body,
|
||||
),
|
||||
for (scope, body) in [
|
||||
("GLOBAL", global_directory_body),
|
||||
("WORKSPACE", empty_page_body),
|
||||
] {
|
||||
Mock::given(method("GET"))
|
||||
.and(path(path_suffix))
|
||||
.and(path("/backend-api/ps/plugins/list"))
|
||||
.and(query_param("scope", scope))
|
||||
.and(query_param("limit", "200"))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_string(body))
|
||||
.mount(&server)
|
||||
.await;
|
||||
}
|
||||
for scope in ["GLOBAL", "WORKSPACE"] {
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/ps/plugins/installed"))
|
||||
.and(query_param("scope", scope))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_string(empty_page_body))
|
||||
.mount(&server)
|
||||
.await;
|
||||
}
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
@@ -161,7 +161,7 @@ async fn plugin_read_reads_remote_plugin_details_when_remote_plugin_enabled() ->
|
||||
)?;
|
||||
|
||||
let detail_body = r#"{
|
||||
"id": "linear@chatgpt-global",
|
||||
"id": "plugins~Plugin_linear",
|
||||
"name": "linear",
|
||||
"scope": "GLOBAL",
|
||||
"installation_policy": "AVAILABLE",
|
||||
@@ -192,7 +192,7 @@ async fn plugin_read_reads_remote_plugin_details_when_remote_plugin_enabled() ->
|
||||
let installed_body = r#"{
|
||||
"plugins": [
|
||||
{
|
||||
"id": "linear@chatgpt-global",
|
||||
"id": "plugins~Plugin_linear",
|
||||
"name": "linear",
|
||||
"scope": "GLOBAL",
|
||||
"installation_policy": "AVAILABLE",
|
||||
@@ -230,7 +230,7 @@ async fn plugin_read_reads_remote_plugin_details_when_remote_plugin_enabled() ->
|
||||
}"#;
|
||||
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/ps/plugins/linear@chatgpt-global"))
|
||||
.and(path("/backend-api/ps/plugins/plugins~Plugin_linear"))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_string(detail_body))
|
||||
@@ -252,7 +252,7 @@ async fn plugin_read_reads_remote_plugin_details_when_remote_plugin_enabled() ->
|
||||
.send_plugin_read_request(PluginReadParams {
|
||||
marketplace_path: None,
|
||||
remote_marketplace_name: Some("chatgpt-global".to_string()),
|
||||
plugin_name: "linear".to_string(),
|
||||
plugin_name: "plugins~Plugin_linear".to_string(),
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -266,7 +266,7 @@ async fn plugin_read_reads_remote_plugin_details_when_remote_plugin_enabled() ->
|
||||
assert_eq!(response.plugin.marketplace_name, "chatgpt-global");
|
||||
assert_eq!(response.plugin.marketplace_path, None);
|
||||
assert_eq!(response.plugin.summary.source, PluginSource::Remote);
|
||||
assert_eq!(response.plugin.summary.id, "linear@chatgpt-global");
|
||||
assert_eq!(response.plugin.summary.id, "plugins~Plugin_linear");
|
||||
assert_eq!(response.plugin.summary.name, "linear");
|
||||
assert_eq!(response.plugin.summary.installed, true);
|
||||
assert_eq!(response.plugin.summary.enabled, false);
|
||||
@@ -300,7 +300,7 @@ async fn plugin_read_maps_missing_remote_plugin_to_invalid_request() -> Result<(
|
||||
)?;
|
||||
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/backend-api/ps/plugins/missing@chatgpt-global"))
|
||||
.and(path("/backend-api/ps/plugins/plugins~Plugin_missing"))
|
||||
.and(header("authorization", "Bearer chatgpt-token"))
|
||||
.and(header("chatgpt-account-id", "account-123"))
|
||||
.respond_with(ResponseTemplate::new(404).set_body_string(r#"{"detail":"not found"}"#))
|
||||
@@ -314,7 +314,7 @@ async fn plugin_read_maps_missing_remote_plugin_to_invalid_request() -> Result<(
|
||||
.send_plugin_read_request(PluginReadParams {
|
||||
marketplace_path: None,
|
||||
remote_marketplace_name: Some("chatgpt-global".to_string()),
|
||||
plugin_name: "missing".to_string(),
|
||||
plugin_name: "plugins~Plugin_missing".to_string(),
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -408,7 +408,11 @@ async fn plugin_read_rejects_invalid_remote_plugin_name() -> Result<()> {
|
||||
|
||||
assert_eq!(err.error.code, -32600);
|
||||
assert!(err.error.message.contains("invalid remote plugin id"));
|
||||
assert!(err.error.message.contains("invalid plugin name"));
|
||||
assert!(
|
||||
err.error
|
||||
.message
|
||||
.contains("only ASCII letters, digits, `_`, `-`, and `~` are allowed")
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@ pub const REMOTE_GLOBAL_MARKETPLACE_DISPLAY_NAME: &str = "ChatGPT Plugins";
|
||||
pub const REMOTE_WORKSPACE_MARKETPLACE_DISPLAY_NAME: &str = "ChatGPT Workspace Plugins";
|
||||
|
||||
const REMOTE_PLUGIN_CATALOG_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
const REMOTE_PLUGIN_LIST_PAGE_LIMIT: u32 = 200;
|
||||
const MAX_REMOTE_DEFAULT_PROMPT_LEN: usize = 128;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -567,6 +568,7 @@ async fn get_remote_plugin_list_page(
|
||||
let client = build_reqwest_client();
|
||||
let mut request = authenticated_request(client.get(&url), auth)?;
|
||||
request = request.query(&[("scope", scope.api_value())]);
|
||||
request = request.query(&[("limit", REMOTE_PLUGIN_LIST_PAGE_LIMIT)]);
|
||||
if let Some(page_token) = page_token {
|
||||
request = request.query(&[("pageToken", page_token)]);
|
||||
}
|
||||
|
||||
@@ -378,6 +378,9 @@
|
||||
"collaboration_modes": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"computer_use": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"connectors": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -2532,6 +2535,9 @@
|
||||
"collaboration_modes": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"computer_use": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"connectors": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -7070,10 +7070,10 @@ async fn feature_requirements_normalize_runtime_feature_mutations() -> std::io::
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn feature_requirements_reject_collab_legacy_alias() {
|
||||
let codex_home = TempDir::new().expect("tempdir");
|
||||
async fn feature_requirements_warn_on_collab_legacy_alias() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
let err = ConfigBuilder::default()
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.cloud_requirements(CloudRequirementsLoader::new(async {
|
||||
Ok(Some(crate::config_loader::ConfigRequirementsToml {
|
||||
@@ -7084,15 +7084,49 @@ async fn feature_requirements_reject_collab_legacy_alias() {
|
||||
}))
|
||||
}))
|
||||
.build()
|
||||
.await
|
||||
.expect_err("legacy aliases should be rejected");
|
||||
.await?;
|
||||
|
||||
assert_eq!(err.kind(), std::io::ErrorKind::InvalidData);
|
||||
assert!(config.features.enabled(Feature::Collab));
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("use canonical feature key `multi_agent`"),
|
||||
"{err}"
|
||||
config.startup_warnings.iter().any(|warning| {
|
||||
warning.contains("Using legacy `features` requirement `collab`")
|
||||
&& warning.contains("prefer canonical feature key `multi_agent`")
|
||||
}),
|
||||
"{:?}",
|
||||
config.startup_warnings
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn feature_requirements_warn_and_ignore_unknown_feature() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.cloud_requirements(CloudRequirementsLoader::new(async {
|
||||
Ok(Some(crate::config_loader::ConfigRequirementsToml {
|
||||
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
|
||||
entries: BTreeMap::from([("made_up_feature".to_string(), true)]),
|
||||
}),
|
||||
..Default::default()
|
||||
}))
|
||||
}))
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
config
|
||||
.startup_warnings
|
||||
.iter()
|
||||
.any(|warning| warning
|
||||
.contains("Ignoring unknown `features` requirement `made_up_feature`")),
|
||||
"{:?}",
|
||||
config.startup_warnings
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -31,13 +31,37 @@ impl ManagedFeatures {
|
||||
pub(crate) fn from_configured(
|
||||
configured_features: Features,
|
||||
feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
) -> std::io::Result<Self> {
|
||||
Self::from_configured_with_optional_warnings(
|
||||
configured_features,
|
||||
feature_requirements,
|
||||
/*startup_warnings*/ None,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn from_configured_with_warnings(
|
||||
configured_features: Features,
|
||||
feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
startup_warnings: &mut Vec<String>,
|
||||
) -> std::io::Result<Self> {
|
||||
Self::from_configured_with_optional_warnings(
|
||||
configured_features,
|
||||
feature_requirements,
|
||||
Some(startup_warnings),
|
||||
)
|
||||
}
|
||||
|
||||
fn from_configured_with_optional_warnings(
|
||||
configured_features: Features,
|
||||
feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
|
||||
startup_warnings: Option<&mut Vec<String>>,
|
||||
) -> std::io::Result<Self> {
|
||||
let (pinned_features, source) = match feature_requirements {
|
||||
Some(Sourced {
|
||||
value: feature_requirements,
|
||||
source,
|
||||
}) => (
|
||||
parse_feature_requirements(feature_requirements, &source)?,
|
||||
parse_feature_requirements(feature_requirements, &source, startup_warnings),
|
||||
Some(source),
|
||||
),
|
||||
None => (BTreeMap::new(), None),
|
||||
@@ -171,7 +195,8 @@ fn feature_requirements_display(feature_requirements: &BTreeMap<Feature, bool>)
|
||||
fn parse_feature_requirements(
|
||||
feature_requirements: FeatureRequirementsToml,
|
||||
source: &RequirementSource,
|
||||
) -> std::io::Result<BTreeMap<Feature, bool>> {
|
||||
mut startup_warnings: Option<&mut Vec<String>>,
|
||||
) -> BTreeMap<Feature, bool> {
|
||||
let mut pinned_features = BTreeMap::new();
|
||||
for (key, enabled) in feature_requirements.entries {
|
||||
if let Some(feature) = canonical_feature_for_key(&key) {
|
||||
@@ -180,22 +205,34 @@ fn parse_feature_requirements(
|
||||
}
|
||||
|
||||
if let Some(feature) = feature_for_key(&key) {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidData,
|
||||
push_feature_requirement_warning(
|
||||
&mut startup_warnings,
|
||||
format!(
|
||||
"invalid `features` requirement `{key}` from {source}: use canonical feature key `{}`",
|
||||
"Using legacy `features` requirement `{key}` from {source}; prefer canonical feature key `{}`",
|
||||
feature.key()
|
||||
),
|
||||
));
|
||||
);
|
||||
pinned_features.insert(feature, enabled);
|
||||
continue;
|
||||
}
|
||||
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidData,
|
||||
format!("invalid `features` requirement `{key}` from {source}"),
|
||||
));
|
||||
push_feature_requirement_warning(
|
||||
&mut startup_warnings,
|
||||
format!("Ignoring unknown `features` requirement `{key}` from {source}"),
|
||||
);
|
||||
}
|
||||
|
||||
Ok(pinned_features)
|
||||
pinned_features
|
||||
}
|
||||
|
||||
fn push_feature_requirement_warning(
|
||||
startup_warnings: &mut Option<&mut Vec<String>>,
|
||||
message: String,
|
||||
) {
|
||||
tracing::warn!("{message}");
|
||||
if let Some(startup_warnings) = startup_warnings.as_deref_mut() {
|
||||
startup_warnings.push(message);
|
||||
}
|
||||
}
|
||||
|
||||
fn explicit_feature_settings_in_config(cfg: &ConfigToml) -> Vec<(String, Feature, bool)> {
|
||||
@@ -272,7 +309,11 @@ pub(crate) fn validate_explicit_feature_settings_in_config_toml(
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let pinned_features = parse_feature_requirements(feature_requirements.clone(), source)?;
|
||||
let pinned_features = parse_feature_requirements(
|
||||
feature_requirements.clone(),
|
||||
source,
|
||||
/*startup_warnings*/ None,
|
||||
);
|
||||
if pinned_features.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -1666,7 +1666,11 @@ impl Config {
|
||||
},
|
||||
feature_overrides,
|
||||
);
|
||||
let features = ManagedFeatures::from_configured(configured_features, feature_requirements)?;
|
||||
let features = ManagedFeatures::from_configured_with_warnings(
|
||||
configured_features,
|
||||
feature_requirements,
|
||||
&mut startup_warnings,
|
||||
)?;
|
||||
let windows_sandbox_mode = resolve_windows_sandbox_mode(&cfg, &config_profile);
|
||||
let windows_sandbox_private_desktop =
|
||||
resolve_windows_sandbox_private_desktop(&cfg, &config_profile);
|
||||
|
||||
@@ -27,8 +27,6 @@ use crate::tools::registry::ToolKind;
|
||||
|
||||
pub struct ToolSuggestHandler;
|
||||
|
||||
const INVALID_SUGGEST_REASON_MESSAGE: &str = "suggest_reason must be a specific one-line user-facing reason for the current request; do not use placeholders like \"placeholder\".";
|
||||
|
||||
impl ToolHandler for ToolSuggestHandler {
|
||||
type Output = FunctionToolOutput;
|
||||
|
||||
@@ -60,8 +58,10 @@ impl ToolHandler for ToolSuggestHandler {
|
||||
|
||||
let args: ToolSuggestArgs = parse_arguments(&arguments)?;
|
||||
let suggest_reason = args.suggest_reason.trim();
|
||||
if let Some(message) = invalid_suggest_reason_message(suggest_reason) {
|
||||
return Err(FunctionCallError::RespondToModel(message.to_string()));
|
||||
if suggest_reason.is_empty() {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"suggest_reason must not be empty".to_string(),
|
||||
));
|
||||
}
|
||||
if args.action_type != DiscoverableToolAction::Install {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
@@ -158,57 +158,6 @@ impl ToolHandler for ToolSuggestHandler {
|
||||
}
|
||||
}
|
||||
|
||||
fn invalid_suggest_reason_message(suggest_reason: &str) -> Option<&'static str> {
|
||||
if suggest_reason.trim().is_empty() {
|
||||
return Some("suggest_reason must not be empty");
|
||||
}
|
||||
|
||||
let normalized = normalize_suggest_reason_for_stub_check(suggest_reason);
|
||||
if normalized.ends_with("goes here")
|
||||
|| matches!(
|
||||
normalized.as_str(),
|
||||
"placeholder"
|
||||
| "placeholder text"
|
||||
| "todo"
|
||||
| "tbd"
|
||||
| "n/a"
|
||||
| "na"
|
||||
| "none"
|
||||
| "null"
|
||||
| "nil"
|
||||
| "reason"
|
||||
| "suggest reason"
|
||||
| "suggestion reason"
|
||||
| "tool suggestion reason"
|
||||
| "concise one-line user-facing reason"
|
||||
| "concise one-line user-facing reason why this tool can help with the current request"
|
||||
| "specific one-line user-facing reason"
|
||||
| "specific one-line user-facing reason why this tool can help with the current request"
|
||||
)
|
||||
{
|
||||
return Some(INVALID_SUGGEST_REASON_MESSAGE);
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
fn normalize_suggest_reason_for_stub_check(suggest_reason: &str) -> String {
|
||||
const WRAPPER_CHARS: &[char] = &[
|
||||
'"', '\'', '`', '<', '>', '[', ']', '(', ')', '{', '}', '.', ',', ':', ';', '-', '_', '*',
|
||||
' ',
|
||||
];
|
||||
|
||||
suggest_reason
|
||||
.trim()
|
||||
.trim_matches(WRAPPER_CHARS)
|
||||
.split_whitespace()
|
||||
.map(|word| word.trim_matches(WRAPPER_CHARS))
|
||||
.filter(|word| !word.is_empty())
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ")
|
||||
.to_ascii_lowercase()
|
||||
}
|
||||
|
||||
async fn verify_tool_suggestion_completed(
|
||||
session: &crate::session::session::Session,
|
||||
turn: &crate::session::turn_context::TurnContext,
|
||||
|
||||
@@ -8,46 +8,6 @@ use crate::plugins::test_support::write_plugins_feature_config;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn invalid_suggest_reason_message_rejects_placeholder_reasons() {
|
||||
for reason in [
|
||||
"",
|
||||
" ",
|
||||
"placeholder",
|
||||
" Placeholder ",
|
||||
"<placeholder>",
|
||||
"[TODO]",
|
||||
"TBD",
|
||||
"n/a",
|
||||
"reason",
|
||||
"suggestion reason",
|
||||
"reason goes here",
|
||||
"Concise one-line user-facing reason why this tool can help with the current request",
|
||||
"Specific one-line user-facing reason why this tool can help with the current request",
|
||||
] {
|
||||
assert!(
|
||||
invalid_suggest_reason_message(reason).is_some(),
|
||||
"expected {reason:?} to be rejected"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_suggest_reason_message_allows_specific_reasons() {
|
||||
for reason in [
|
||||
"Search Slack messages related to the release plan.",
|
||||
"Find and reference issues in GitHub for this bug report.",
|
||||
"Use Google Calendar to compare attendee availability.",
|
||||
"Use Figma to inspect placeholder text in the selected design.",
|
||||
] {
|
||||
assert_eq!(
|
||||
invalid_suggest_reason_message(reason),
|
||||
None,
|
||||
"expected {reason:?} to be allowed"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
|
||||
@@ -6,11 +6,6 @@ Use this ONLY when:
|
||||
- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.
|
||||
- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.
|
||||
- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.
|
||||
- The `suggest_reason` must be a specific one-line user-facing reason for the current request. Do not use placeholders like `placeholder`.
|
||||
|
||||
There are two types of allowed suggestions:
|
||||
1. Suggest a plugin needed in the context that explicitly and unambiguously fits the user intent but is not installed, tool_type = "plugin", action_type = "install"
|
||||
2. Suggest a connector needed in the context but not installed (even when its plugin is installed), tool_type = "connector", action_type = "install"
|
||||
|
||||
Tool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.
|
||||
|
||||
@@ -19,13 +14,13 @@ Discoverable tools:
|
||||
|
||||
Workflow:
|
||||
|
||||
1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent. If tool search is available, tool search should happen before tool suggestion.
|
||||
2. If no available or searchable tool is found, match the user's intent against the discoverable tools list above. Decide if any of the discoverable tools match the user intent explicitly and unambiguously. Suggest a tool only when it qualifies all the conditions.
|
||||
1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.
|
||||
2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.
|
||||
3. If one tool clearly fits, call `tool_suggest` with:
|
||||
- `tool_type`: `connector` or `plugin`
|
||||
- `action_type`: `install` or `enable`
|
||||
- `tool_id`: exact id from the discoverable tools list above
|
||||
- `suggest_reason`: concise one-line user-facing reason this tool can help with the current request, must not be empty and must not be a placeholder
|
||||
- `suggest_reason`: concise one-line user-facing reason this tool can help with the current request
|
||||
4. After the suggestion flow completes:
|
||||
- if the user finished the install or enable flow, continue by searching again or using the newly available tool
|
||||
- if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks you to.
|
||||
|
||||
@@ -164,6 +164,10 @@ pub enum Feature {
|
||||
///
|
||||
/// Requirements-only gate: this should be set from requirements, not user config.
|
||||
BrowserUse,
|
||||
/// Allow Codex Computer Use.
|
||||
///
|
||||
/// Requirements-only gate: this should be set from requirements, not user config.
|
||||
ComputerUse,
|
||||
/// Temporary internal-only flag for PS-backed remote plugin catalog development.
|
||||
RemotePlugin,
|
||||
/// Show the startup prompt for migrating external agent config into Codex.
|
||||
@@ -868,6 +872,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ComputerUse,
|
||||
key: "computer_use",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::RemotePlugin,
|
||||
key: "remote_plugin",
|
||||
|
||||
@@ -153,6 +153,10 @@ fn browser_controls_are_stable_and_enabled_by_default() {
|
||||
assert_eq!(Feature::BrowserUse.stage(), Stage::Stable);
|
||||
assert_eq!(Feature::BrowserUse.default_enabled(), true);
|
||||
assert_eq!(feature_for_key("browser_use"), Some(Feature::BrowserUse));
|
||||
|
||||
assert_eq!(Feature::ComputerUse.stage(), Stage::Stable);
|
||||
assert_eq!(Feature::ComputerUse.default_enabled(), true);
|
||||
assert_eq!(feature_for_key("computer_use"), Some(Feature::ComputerUse));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -272,6 +272,11 @@ pub fn collect_tool_search_source_infos<'a>(
|
||||
}
|
||||
|
||||
pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> ToolSpec {
|
||||
let discoverable_tool_ids = discoverable_tools
|
||||
.iter()
|
||||
.map(|tool| tool.id.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
let properties = BTreeMap::from([
|
||||
(
|
||||
"tool_type".to_string(),
|
||||
@@ -288,14 +293,14 @@ pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> Tool
|
||||
),
|
||||
(
|
||||
"tool_id".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Connector or plugin id to suggest. Must be one of the discoverable tool ids.".to_string(),
|
||||
)),
|
||||
JsonSchema::string(Some(format!(
|
||||
"Connector or plugin id to suggest. Must be one of: {discoverable_tool_ids}."
|
||||
))),
|
||||
),
|
||||
(
|
||||
"suggest_reason".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Concise one-line user-facing reason why this tool can help with the current request, must not be empty and must not be a placeholder."
|
||||
"Concise one-line user-facing reason why this tool can help with the current request."
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
@@ -303,7 +308,7 @@ pub fn create_tool_suggest_tool(discoverable_tools: &[ToolSuggestEntry]) -> Tool
|
||||
|
||||
let discoverable_tools = format_discoverable_tools(discoverable_tools);
|
||||
let description = format!(
|
||||
"# Tool suggestion discovery\n\nSuggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.\n\nUse this ONLY when:\n- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `{TOOL_SEARCH_TOOL_NAME}` (if available) and other means.\n- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.\n- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.\n- The `suggest_reason` must be a specific one-line user-facing reason for the current request. Do not use placeholders like `placeholder`.\n\nThere are two types of allowed suggestions:\n1. Suggest a plugin needed in the context that explicitly and unambiguously fits the user intent but is not installed, tool_type = \"plugin\", action_type = \"install\"\n2. Suggest a connector needed in the context but not installed (even when its plugin is installed), tool_type = \"connector\", action_type = \"install\"\n\nTool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.\n\nDiscoverable tools:\n{discoverable_tools}\n\nWorkflow:\n\n1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent. If tool search is available, tool search should happen before tool suggestion.\n2. If no available or searchable tool is found, match the user's intent against the discoverable tools list above. Decide if any of the discoverable tools match the user intent explicitly and unambiguously. Suggest a tool only when it qualifies all the conditions.\n3. If one tool clearly fits, call `{TOOL_SUGGEST_TOOL_NAME}` with:\n - `tool_type`: `connector` or `plugin`\n - `action_type`: `install` or `enable`\n - `tool_id`: exact id from the discoverable tools list above\n - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request, must not be empty and must not be a placeholder\n4. After the suggestion flow completes:\n - if the user finished the install or enable flow, continue by searching again or using the newly available tool\n - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks for it."
|
||||
"# Tool suggestion discovery\n\nSuggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.\n\nUse this ONLY when:\n- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `{TOOL_SEARCH_TOOL_NAME}` (if available) and other means.\n- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.\n- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.\n\nTool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.\n\nDiscoverable tools:\n{discoverable_tools}\n\nWorkflow:\n\n1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.\n2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.\n3. If one tool clearly fits, call `{TOOL_SUGGEST_TOOL_NAME}` with:\n - `tool_type`: `connector` or `plugin`\n - `action_type`: `install` or `enable`\n - `tool_id`: exact id from the discoverable tools list above\n - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request\n4. After the suggestion flow completes:\n - if the user finished the install or enable flow, continue by searching again or using the newly available tool\n - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks for it."
|
||||
);
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
|
||||
@@ -50,100 +50,69 @@ fn create_tool_search_tool_deduplicates_and_renders_enabled_sources() {
|
||||
|
||||
#[test]
|
||||
fn create_tool_suggest_tool_uses_plugin_summary_fallback() {
|
||||
let tool = create_tool_suggest_tool(&[
|
||||
ToolSuggestEntry {
|
||||
id: "slack@openai-curated".to_string(),
|
||||
name: "Slack".to_string(),
|
||||
description: None,
|
||||
tool_type: DiscoverableToolType::Connector,
|
||||
has_skills: false,
|
||||
mcp_server_names: Vec::new(),
|
||||
app_connector_ids: Vec::new(),
|
||||
},
|
||||
ToolSuggestEntry {
|
||||
id: "github".to_string(),
|
||||
name: "GitHub".to_string(),
|
||||
description: None,
|
||||
tool_type: DiscoverableToolType::Plugin,
|
||||
has_skills: true,
|
||||
mcp_server_names: vec!["github-mcp".to_string()],
|
||||
app_connector_ids: vec!["github-app".to_string()],
|
||||
},
|
||||
]);
|
||||
let ToolSpec::Function(ResponsesApiTool {
|
||||
name,
|
||||
description,
|
||||
strict,
|
||||
defer_loading,
|
||||
parameters,
|
||||
output_schema,
|
||||
}) = tool
|
||||
else {
|
||||
panic!("expected function tool");
|
||||
};
|
||||
|
||||
assert_eq!(name, "tool_suggest");
|
||||
assert!(!strict);
|
||||
assert_eq!(defer_loading, None);
|
||||
assert_eq!(output_schema, None);
|
||||
assert!(
|
||||
description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request"
|
||||
)
|
||||
);
|
||||
assert!(description.contains("This includes `tool_search` (if available) and other means."));
|
||||
assert!(description.contains("There are two types of allowed suggestions:"));
|
||||
assert!(description.contains("tool_type = \"plugin\", action_type = \"install\""));
|
||||
assert!(description.contains("tool_type = \"connector\", action_type = \"install\""));
|
||||
assert!(
|
||||
description.contains("- GitHub (id: `github`, type: plugin, action: install): skills; MCP servers: github-mcp; app connectors: github-app")
|
||||
);
|
||||
assert!(
|
||||
description.contains("- Slack (id: `slack@openai-curated`, type: connector, action: install): No description provided.")
|
||||
);
|
||||
assert!(description.contains("placeholders like `placeholder`"));
|
||||
|
||||
assert_eq!(
|
||||
parameters,
|
||||
JsonSchema::object(
|
||||
BTreeMap::from([
|
||||
(
|
||||
"action_type".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Suggested action for the tool. Use \"install\" or \"enable\"."
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
(
|
||||
"suggest_reason".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Concise one-line user-facing reason why this tool can help with the current request, must not be empty and must not be a placeholder."
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
(
|
||||
"tool_id".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Connector or plugin id to suggest. Must be one of the discoverable tool ids."
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
(
|
||||
create_tool_suggest_tool(&[
|
||||
ToolSuggestEntry {
|
||||
id: "slack@openai-curated".to_string(),
|
||||
name: "Slack".to_string(),
|
||||
description: None,
|
||||
tool_type: DiscoverableToolType::Connector,
|
||||
has_skills: false,
|
||||
mcp_server_names: Vec::new(),
|
||||
app_connector_ids: Vec::new(),
|
||||
},
|
||||
ToolSuggestEntry {
|
||||
id: "github".to_string(),
|
||||
name: "GitHub".to_string(),
|
||||
description: None,
|
||||
tool_type: DiscoverableToolType::Plugin,
|
||||
has_skills: true,
|
||||
mcp_server_names: vec!["github-mcp".to_string()],
|
||||
app_connector_ids: vec!["github-app".to_string()],
|
||||
},
|
||||
]),
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "tool_suggest".to_string(),
|
||||
description: "# Tool suggestion discovery\n\nSuggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin, when the user clearly wants a capability that is not currently available in the active `tools` list.\n\nUse this ONLY when:\n- You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means.\n- For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely.\n- For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself. Do not suggest a plugin just because one of its connectors or capabilities seems relevant.\n\nTool suggestions should only use the discoverable tools listed here. DO NOT explore or recommend tools that are not on this list.\n\nDiscoverable tools:\n- GitHub (id: `github`, type: plugin, action: install): skills; MCP servers: github-mcp; app connectors: github-app\n- Slack (id: `slack@openai-curated`, type: connector, action: install): No description provided.\n\nWorkflow:\n\n1. Ensure all possible means have been exhausted to find an existing available tool but none of them matches the request intent.\n2. Match the user's request against the discoverable tools list above. Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard.\n3. If one tool clearly fits, call `tool_suggest` with:\n - `tool_type`: `connector` or `plugin`\n - `action_type`: `install` or `enable`\n - `tool_id`: exact id from the discoverable tools list above\n - `suggest_reason`: concise one-line user-facing reason this tool can help with the current request\n4. After the suggestion flow completes:\n - if the user finished the install or enable flow, continue by searching again or using the newly available tool\n - if the user did not finish, continue without that tool, and don't suggest that tool again unless the user explicitly asks for it.".to_string(),
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::object(BTreeMap::from([
|
||||
(
|
||||
"action_type".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Suggested action for the tool. Use \"install\" or \"enable\"."
|
||||
.to_string(),
|
||||
),),
|
||||
),
|
||||
(
|
||||
"suggest_reason".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Concise one-line user-facing reason why this tool can help with the current request."
|
||||
.to_string(),
|
||||
),),
|
||||
),
|
||||
(
|
||||
"tool_id".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Connector or plugin id to suggest. Must be one of: slack@openai-curated, github."
|
||||
.to_string(),
|
||||
),),
|
||||
),
|
||||
(
|
||||
"tool_type".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"."
|
||||
.to_string(),
|
||||
),),
|
||||
),
|
||||
]), Some(vec![
|
||||
"tool_type".to_string(),
|
||||
JsonSchema::string(Some(
|
||||
"Type of discoverable tool to suggest. Use \"connector\" or \"plugin\"."
|
||||
.to_string(),
|
||||
)),
|
||||
),
|
||||
]),
|
||||
Some(vec![
|
||||
"tool_type".to_string(),
|
||||
"action_type".to_string(),
|
||||
"tool_id".to_string(),
|
||||
"suggest_reason".to_string(),
|
||||
]),
|
||||
Some(false.into()),
|
||||
)
|
||||
"action_type".to_string(),
|
||||
"tool_id".to_string(),
|
||||
"suggest_reason".to_string(),
|
||||
]), Some(false.into())),
|
||||
output_schema: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -1575,7 +1575,7 @@ fn tool_suggest_can_be_registered_without_search_tool() {
|
||||
"Suggests a missing connector in an installed plugin, or in narrower cases a not installed but discoverable plugin"
|
||||
));
|
||||
assert!(description.contains(
|
||||
"If tool search is available, tool search should happen before tool suggestion."
|
||||
"You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means."
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1646,22 +1646,27 @@ fn tool_suggest_description_lists_discoverable_tools() {
|
||||
assert!(description.contains("Plan events and schedules."));
|
||||
assert!(description.contains("Find and summarize email threads."));
|
||||
assert!(description.contains("id: `sample@test`, type: plugin, action: install"));
|
||||
assert!(description.contains("tool_type = \"plugin\", action_type = \"install\""));
|
||||
assert!(description.contains("tool_type = \"connector\", action_type = \"install\""));
|
||||
assert!(description.contains("`action_type`: `install` or `enable`"));
|
||||
assert!(
|
||||
description.contains("skills; MCP servers: sample-docs; app connectors: connector_sample")
|
||||
);
|
||||
assert!(
|
||||
description.contains(
|
||||
"You've already tried to find a matching available tool for the user's request but couldn't find a good match. This includes `tool_search` (if available) and other means."
|
||||
)
|
||||
);
|
||||
assert!(description.contains(
|
||||
"If tool search is available, tool search should happen before tool suggestion."
|
||||
));
|
||||
assert!(description.contains("There are two types of allowed suggestions:"));
|
||||
assert!(description.contains(
|
||||
"Suggest a plugin needed in the context that explicitly and unambiguously fits the user intent but is not installed"
|
||||
"For connectors/apps that are not installed but needed for an installed plugin, suggest to install them if the task requirements match precisely."
|
||||
));
|
||||
assert!(description.contains(
|
||||
"Suggest a connector needed in the context but not installed (even when its plugin is installed)"
|
||||
"For plugins that are not installed but discoverable, only suggest discoverable and installable plugins when the user's intent very explicitly and unambiguously matches that plugin itself."
|
||||
));
|
||||
assert!(description.contains(
|
||||
"Do not suggest a plugin just because one of its connectors or capabilities seems relevant."
|
||||
));
|
||||
assert!(description.contains(
|
||||
"Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard."
|
||||
));
|
||||
assert!(description.contains("Suggest a tool only when it qualifies all the conditions."));
|
||||
assert!(description.contains("DO NOT explore or recommend tools that are not on this list."));
|
||||
assert!(!description.contains("{{discoverable_tools}}"));
|
||||
assert!(!description.contains("tool_search fails to find a good match"));
|
||||
|
||||
@@ -311,6 +311,8 @@ impl App {
|
||||
|| approvals_reviewer_override.is_some()
|
||||
|| sandbox_policy_override.is_some()
|
||||
{
|
||||
self.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
// This uses `OverrideTurnContext` intentionally: toggling the
|
||||
// experiment should update the active thread's effective approval
|
||||
// settings immediately, just like a `/approvals` selection. Without
|
||||
|
||||
@@ -1134,6 +1134,8 @@ impl App {
|
||||
Some(self.config.permissions.approval_policy.value());
|
||||
self.chat_widget
|
||||
.set_approval_policy(self.config.permissions.approval_policy.value());
|
||||
self.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
}
|
||||
AppEvent::UpdateSandboxPolicy(policy) => {
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -1162,6 +1164,8 @@ impl App {
|
||||
}
|
||||
self.runtime_sandbox_policy_override =
|
||||
Some(self.config.permissions.sandbox_policy.get().clone());
|
||||
self.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
|
||||
// If sandbox policy becomes workspace-write or read-only, run the Windows world-writable scan.
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -1196,6 +1200,8 @@ impl App {
|
||||
AppEvent::UpdateApprovalsReviewer(policy) => {
|
||||
self.config.approvals_reviewer = policy;
|
||||
self.chat_widget.set_approvals_reviewer(policy);
|
||||
self.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
let profile = self.active_profile.as_deref();
|
||||
let segments = if let Some(profile) = profile {
|
||||
vec![
|
||||
|
||||
@@ -5,6 +5,34 @@ use codex_app_server_protocol::Thread;
|
||||
use codex_protocol::ThreadId;
|
||||
|
||||
impl App {
|
||||
pub(super) async fn sync_active_thread_permission_settings_to_cached_session(&mut self) {
|
||||
let Some(active_thread_id) = self.active_thread_id else {
|
||||
return;
|
||||
};
|
||||
|
||||
let approval_policy = self.config.permissions.approval_policy.value();
|
||||
let approvals_reviewer = self.config.approvals_reviewer;
|
||||
let sandbox_policy = self.config.permissions.sandbox_policy.get().clone();
|
||||
let update_session = |session: &mut ThreadSessionState| {
|
||||
session.approval_policy = approval_policy;
|
||||
session.approvals_reviewer = approvals_reviewer;
|
||||
session.sandbox_policy = sandbox_policy.clone();
|
||||
};
|
||||
|
||||
if self.primary_thread_id == Some(active_thread_id)
|
||||
&& let Some(session) = self.primary_session_configured.as_mut()
|
||||
{
|
||||
update_session(session);
|
||||
}
|
||||
|
||||
if let Some(channel) = self.thread_event_channels.get(&active_thread_id) {
|
||||
let mut store = channel.store.lock().await;
|
||||
if let Some(session) = store.session.as_mut() {
|
||||
update_session(session);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn session_state_for_thread_read(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
@@ -53,3 +81,119 @@ impl App {
|
||||
session
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::app::side::SideThreadState;
|
||||
use crate::app::test_support::make_test_app;
|
||||
use crate::app::thread_events::ThreadEventChannel;
|
||||
use crate::test_support::PathBufExt;
|
||||
use crate::test_support::test_path_buf;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::PathBuf;
|
||||
|
||||
fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState {
|
||||
ThreadSessionState {
|
||||
thread_id,
|
||||
forked_from_id: None,
|
||||
fork_parent_title: None,
|
||||
thread_name: None,
|
||||
model: "gpt-test".to_string(),
|
||||
model_provider_id: "test-provider".to_string(),
|
||||
service_tier: None,
|
||||
approval_policy: AskForApproval::Never,
|
||||
approvals_reviewer: ApprovalsReviewer::User,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
permission_profile: None,
|
||||
cwd: cwd.abs(),
|
||||
instruction_source_paths: Vec::new(),
|
||||
reasoning_effort: None,
|
||||
history_log_id: 0,
|
||||
history_entry_count: 0,
|
||||
network_proxy: None,
|
||||
rollout_path: Some(PathBuf::new()),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn permission_settings_sync_updates_active_snapshot_without_rewriting_side_thread() {
|
||||
let mut app = make_test_app().await;
|
||||
let main_thread_id =
|
||||
ThreadId::from_string("00000000-0000-0000-0000-000000000401").expect("valid thread");
|
||||
let side_thread_id =
|
||||
ThreadId::from_string("00000000-0000-0000-0000-000000000402").expect("valid thread");
|
||||
let main_session = test_thread_session(main_thread_id, test_path_buf("/tmp/main"));
|
||||
let side_session = ThreadSessionState {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
|
||||
..test_thread_session(side_thread_id, test_path_buf("/tmp/side"))
|
||||
};
|
||||
|
||||
app.primary_thread_id = Some(main_thread_id);
|
||||
app.active_thread_id = Some(main_thread_id);
|
||||
app.primary_session_configured = Some(main_session.clone());
|
||||
app.thread_event_channels.insert(
|
||||
main_thread_id,
|
||||
ThreadEventChannel::new_with_session(
|
||||
/*capacity*/ 4,
|
||||
main_session.clone(),
|
||||
Vec::new(),
|
||||
),
|
||||
);
|
||||
app.thread_event_channels.insert(
|
||||
side_thread_id,
|
||||
ThreadEventChannel::new_with_session(
|
||||
/*capacity*/ 4,
|
||||
side_session.clone(),
|
||||
Vec::new(),
|
||||
),
|
||||
);
|
||||
app.side_threads
|
||||
.insert(side_thread_id, SideThreadState::new(main_thread_id));
|
||||
app.config.permissions.approval_policy =
|
||||
codex_config::Constrained::allow_any(AskForApproval::OnRequest);
|
||||
app.config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
app.config.permissions.sandbox_policy =
|
||||
codex_config::Constrained::allow_any(SandboxPolicy::new_workspace_write_policy());
|
||||
|
||||
app.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
|
||||
let expected_main_session = ThreadSessionState {
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
approvals_reviewer: ApprovalsReviewer::AutoReview,
|
||||
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
|
||||
..main_session
|
||||
};
|
||||
assert_eq!(
|
||||
app.primary_session_configured,
|
||||
Some(expected_main_session.clone())
|
||||
);
|
||||
|
||||
let main_store_session = app
|
||||
.thread_event_channels
|
||||
.get(&main_thread_id)
|
||||
.expect("main thread channel")
|
||||
.store
|
||||
.lock()
|
||||
.await
|
||||
.session
|
||||
.clone();
|
||||
assert_eq!(main_store_session, Some(expected_main_session));
|
||||
|
||||
let side_store_session = app
|
||||
.thread_event_channels
|
||||
.get(&side_thread_id)
|
||||
.expect("side thread channel")
|
||||
.store
|
||||
.lock()
|
||||
.await
|
||||
.session
|
||||
.clone();
|
||||
assert_eq!(side_store_session, Some(side_session));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user