Merge b53ebc2774 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-04-22 23:02:29 -07:00
committed by GitHub
26 changed files with 360 additions and 61 deletions
-1
View File
@@ -1859,7 +1859,6 @@ dependencies = [
"codex-model-provider-info",
"codex-models-manager",
"codex-otel",
"codex-plugin",
"codex-protocol",
"codex-rmcp-client",
"codex-rollout",
@@ -25,6 +25,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -34,6 +35,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -25,6 +25,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -34,6 +35,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -25,6 +25,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -34,6 +35,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -84,6 +84,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -93,6 +94,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -25,6 +25,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -34,6 +35,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -5298,6 +5298,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
@@ -5307,6 +5308,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
@@ -148,6 +148,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -157,6 +158,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -25,6 +25,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -34,6 +35,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -25,6 +25,7 @@
]
},
"read": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -34,6 +35,7 @@
]
},
"write": {
"description": "This will be removed in favor of `entries`.",
"items": {
"$ref": "#/definitions/AbsolutePathBuf"
},
@@ -4,4 +4,12 @@
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { FileSystemSandboxEntry } from "./FileSystemSandboxEntry";
export type AdditionalFileSystemPermissions = { read: Array<AbsolutePathBuf> | null, write: Array<AbsolutePathBuf> | null, globScanMaxDepth?: number, entries?: Array<FileSystemSandboxEntry>, };
export type AdditionalFileSystemPermissions = {
/**
* This will be removed in favor of `entries`.
*/
read: Array<AbsolutePathBuf> | null,
/**
* This will be removed in favor of `entries`.
*/
write: Array<AbsolutePathBuf> | null, globScanMaxDepth?: number, entries?: Array<FileSystemSandboxEntry>, };
@@ -1273,7 +1273,9 @@ impl From<CoreNetworkApprovalContext> for NetworkApprovalContext {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct AdditionalFileSystemPermissions {
/// This will be removed in favor of `entries`.
pub read: Option<Vec<AbsolutePathBuf>>,
/// This will be removed in favor of `entries`.
pub write: Option<Vec<AbsolutePathBuf>>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
@@ -1286,11 +1288,26 @@ pub struct AdditionalFileSystemPermissions {
impl From<CoreFileSystemPermissions> for AdditionalFileSystemPermissions {
fn from(value: CoreFileSystemPermissions) -> Self {
if let Some((read, write)) = value.legacy_read_write_roots() {
let mut entries = Vec::with_capacity(
read.as_ref().map_or(0, Vec::len) + write.as_ref().map_or(0, Vec::len),
);
if let Some(paths) = read.as_ref() {
entries.extend(paths.iter().map(|path| FileSystemSandboxEntry {
path: FileSystemPath::Path { path: path.clone() },
access: FileSystemAccessMode::Read,
}));
}
if let Some(paths) = write.as_ref() {
entries.extend(paths.iter().map(|path| FileSystemSandboxEntry {
path: FileSystemPath::Path { path: path.clone() },
access: FileSystemAccessMode::Write,
}));
}
Self {
read,
write,
glob_scan_max_depth: None,
entries: None,
entries: Some(entries),
}
} else {
Self {
-1
View File
@@ -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(())
}
+2
View File
@@ -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)]);
}
+6
View File
@@ -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"
},
+43 -9
View File
@@ -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]
+53 -12
View File
@@ -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(());
}
+5 -1
View File
@@ -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);
+10
View File
@@ -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",
+4
View File
@@ -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]
@@ -314,6 +314,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
+6
View File
@@ -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,118 @@ 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(),
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::GuardianSubagent;
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::GuardianSubagent,
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));
}
}