github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-21 05:22:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

Validate step settings against proposed permissions (#40647)

Browse Source 
## Why

Model requirements must be evaluated against the permissions that will apply
after a settings update. Validating against the current profile can reject a
valid model-and-permission update or allow a later permission-only update to
invalidate the selected model.

## What changed

- Group model, reasoning, service-tier, personality, and approval inputs into
  `StepSettings`, with a single apply-and-validate path.
- Apply permission and environment changes before validating step settings, so
  auto-review requirements use the proposed effective permissions.
- Revalidate existing step settings when permission or environment changes alter
  their constraints, and commit the update only when the complete candidate is
  valid.

## Testing

Added unit and integration coverage for atomic model/permission updates,
permission-only revalidation, environment-derived constraints, and both
standalone and turn-start settings operations.

GitOrigin-RevId: f50dfbd300484a09acc51ee4607278f300ca1cd9
This commit is contained in:
joeflorencio-openai
2026-08-25 17:27:04 +00:00
committed by copyberry
parent 4e883ce36e
commit 1d64085e67
15 changed files with 932 additions and 236 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex_thread.rs
+9 -6
View File
@@ -6,6 +6,7 @@ use crate::session::SessionIo;
use crate::session::SessionSettingsUpdate;
use crate::session::new_submission_id;
use crate::session::session::Session;
use crate::session::step_settings::StepSettingsUpdate;
use codex_diagnostics::Gauge;
use codex_diagnostics::GaugeGuard;
use codex_exec_server::SelectedCapabilityRootsStatus;
@@ -567,18 +568,20 @@ impl CodexThread {
};
SessionSettingsUpdate {
step_settings: StepSettingsUpdate {
collaboration_mode: Some(collaboration_mode),
reasoning_summary: summary,
service_tier,
personality,
approval_policy,
approvals_reviewer,
},
environments,
profile_workspace_roots,
approval_policy,
approvals_reviewer,
sandbox_policy,
permission_profile,
active_permission_profile,
windows_sandbox_level,
collaboration_mode: Some(collaboration_mode),
reasoning_summary: summary,
service_tier,
personality,
..Default::default()
}
}
codex-rs/core/src/session/environment.rs
+9 -2
View File
@@ -20,14 +20,21 @@ use crate::session::session::SessionSettingsUpdate;
pub(super) fn validate_environment_selections(
selections: &[TurnEnvironmentSelection],
) -> CodexResult<()> {
) -> ConstraintResult<()> {
for selection in selections {
match &selection.config {
EnvironmentConfigState::FromThread
| EnvironmentConfigState::Pending
| EnvironmentConfigState::Failed(_) => {}
EnvironmentConfigState::Ready(config) => {
validate_environment_config(selection, config)?;
validate_environment_config(selection, config).map_err(|error| {
ConstraintError::InvalidValue {
field_name: "environments",
candidate: "environment configuration".to_string(),
allowed: format!("valid environment configuration ({error})"),
requirement_source: codex_config::RequirementSource::Unknown,
}
})?;
}
}
}
codex-rs/core/src/session/mcp_runtime.rs
+3 -2
View File
@@ -41,8 +41,9 @@ impl Session {
// This gap predates that API: an internal Windows-level-only settings update
// can leave the published MCP configuration stale.
current.cwd() != next.cwd()
|| current.approval_policy.value() != next.approval_policy.value()
|| current.approvals_reviewer != next.approvals_reviewer
|| current.step_settings.approval_policy.value()
!= next.step_settings.approval_policy.value()
|| current.step_settings.approvals_reviewer != next.step_settings.approvals_reviewer
|| current.permission_profile() != next.permission_profile()
|| updates.environments.as_ref().is_some_and(|environments| {
environments.environments != self.services.turn_environments.selections()
codex-rs/core/src/session/mod.rs
+16 -8
View File
@@ -41,6 +41,7 @@ use crate::image_preparation::unified_image_budget_enabled;
use crate::parse_turn_item;
use crate::realtime_conversation::RealtimeConversationManager;
use crate::session::step_context::StepContext;
use crate::session::step_settings::StepSettings;
use crate::session::turn_context::TurnEnvironment;
use crate::session_prefix::format_inter_agent_completion_message;
use crate::skills_load_input_from_config;
@@ -193,7 +194,6 @@ use crate::codex_thread::ThreadConfigSnapshot;
#[cfg(test)]
use crate::compact::collect_user_messages;
use crate::config::Config;
use crate::config::Constrained;
use crate::config::ConstraintResult;
use crate::config::PermissionProfileSnapshot;
use crate::config::PermissionProfileState;
@@ -230,6 +230,7 @@ mod rollout_reconstruction;
#[allow(clippy::module_inception)]
pub(crate) mod session;
pub(crate) mod step_context;
pub(crate) mod step_settings;
mod thread_settings;
pub(crate) mod time_reminder;
mod token_budget;
@@ -705,14 +706,17 @@ impl Session {
config.model_provider.clone(),
Some(Arc::clone(&auth_manager)),
),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
service_tier,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
personality: config.personality,
base_instructions,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: session_permission_profile_state_from_config(&config)?,
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -3517,7 +3521,11 @@ impl Session {
pub(crate) async fn collaboration_mode(&self) -> CollaborationMode {
let state = self.state.lock().await;
state.session_configuration.collaboration_mode.clone()
state
.session_configuration
.step_settings
.collaboration_mode
.clone()
}
pub(crate) fn multi_agent_version(&self) -> Option<MultiAgentVersion> {
codex-rs/core/src/session/review.rs
+1
View File
@@ -93,6 +93,7 @@ pub(super) async fn spawn_review_thread(
state.session_configuration.thread_source.clone(),
state
.session_configuration
.step_settings
.service_tier
.clone()
.or_else(|| config.service_tier.clone()),
codex-rs/core/src/session/session.rs
+62 -132
View File
@@ -1,5 +1,9 @@
use super::input_queue::InputQueue;
use super::mcp_refresh::McpRefresh;
use super::step_settings::ModelInfoOverrides;
use super::step_settings::StepSettings;
use super::step_settings::StepSettingsConstraints;
use super::step_settings::StepSettingsUpdate;
use super::*;
use crate::agents_md_manager::AgentsMdManager;
use crate::config::ConstraintError;
@@ -17,8 +21,6 @@ use codex_login::auth::AgentIdentityAuthPolicy;
use codex_model_provider::SharedModelProvider;
use codex_protocol::SessionId;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::config_types::ShellEnvironmentPolicy;
use codex_protocol::mcp::ClientMcpExtensions;
use codex_protocol::permissions::FileSystemPath;
@@ -75,22 +77,17 @@ pub(crate) struct SessionConfiguration {
/// Runtime provider and its provider-specific execution policy.
pub(super) provider: SharedModelProvider,
pub(super) collaboration_mode: CollaborationMode,
pub(super) model_reasoning_summary: Option<ReasoningSummaryConfig>,
pub(super) service_tier: Option<String>,
/// Desired configured inputs inherited by future turns.
pub(super) step_settings: Arc<StepSettings>,
/// Explicit startup overrides used when resolving effective model metadata.
pub(super) model_info_overrides: ModelInfoOverrides,
/// Developer instructions that supplement the base instructions.
pub(super) developer_instructions: Option<String>,
/// Personality preference for the model.
pub(super) personality: Option<Personality>,
/// Base instructions for the session.
pub(super) base_instructions: String,
/// When to escalate for approval for execution
pub(super) approval_policy: Constrained<AskForApproval>,
pub(super) approvals_reviewer: ApprovalsReviewer,
/// Permission profile state for the session. Keep the constrained profile,
/// active profile id, and profile-defined workspace roots in sync by using
/// the methods below instead of mutating the fields independently.
@@ -226,11 +223,11 @@ impl SessionConfiguration {
.map(|config| config.permission_profile.clone())
.unwrap_or_else(|| self.permission_profile_state.snapshot());
ThreadConfigSnapshot {
model: self.collaboration_mode.model().to_string(),
model: self.step_settings.collaboration_mode.model().to_string(),
model_provider_id: self.original_config_do_not_use.model_provider_id.clone(),
service_tier: self.service_tier.clone(),
approval_policy: self.approval_policy.value(),
approvals_reviewer: self.approvals_reviewer,
service_tier: self.step_settings.service_tier.clone(),
approval_policy: self.step_settings.approval_policy.value(),
approvals_reviewer: self.step_settings.approvals_reviewer,
permission_profile: permission_profile
.permission_profile()
.clone()
@@ -243,10 +240,10 @@ impl SessionConfiguration {
workspace_roots,
profile_workspace_roots: permission_profile.profile_workspace_roots().to_vec(),
ephemeral: self.original_config_do_not_use.ephemeral,
reasoning_effort: self.collaboration_mode.reasoning_effort(),
reasoning_summary: self.model_reasoning_summary,
personality: self.personality,
collaboration_mode: self.collaboration_mode.clone(),
reasoning_effort: self.step_settings.collaboration_mode.reasoning_effort(),
reasoning_summary: self.step_settings.reasoning_summary,
personality: self.step_settings.personality,
collaboration_mode: self.step_settings.collaboration_mode.clone(),
session_source: self.session_source.clone(),
history_mode: self.history_mode,
forked_from_thread_id: self.forked_from_thread_id,
@@ -262,18 +259,18 @@ impl SessionConfiguration {
environment_selections: &[TurnEnvironmentSelection],
) -> ThreadSettingsSnapshot {
ThreadSettingsSnapshot {
model: self.collaboration_mode.model().to_string(),
model: self.step_settings.collaboration_mode.model().to_string(),
model_provider_id: self.original_config_do_not_use.model_provider_id.clone(),
service_tier: self.service_tier.clone(),
approval_policy: self.approval_policy.value(),
approvals_reviewer: self.approvals_reviewer,
service_tier: self.step_settings.service_tier.clone(),
approval_policy: self.step_settings.approval_policy.value(),
approvals_reviewer: self.step_settings.approvals_reviewer,
permission_profile: self.materialized_permission_profile(environment_selections),
active_permission_profile: self.active_permission_profile(),
cwd: self.legacy_fallback_cwd.clone(),
reasoning_effort: self.collaboration_mode.reasoning_effort(),
reasoning_summary: self.model_reasoning_summary,
personality: self.personality,
collaboration_mode: self.collaboration_mode.clone(),
reasoning_effort: self.step_settings.collaboration_mode.reasoning_effort(),
reasoning_summary: self.step_settings.reasoning_summary,
personality: self.step_settings.personality,
collaboration_mode: self.step_settings.collaboration_mode.clone(),
}
}
@@ -292,14 +289,14 @@ impl SessionConfiguration {
.profile_workspace_roots()
.to_vec(),
),
approval_policy: Some(self.approval_policy.value()),
approvals_reviewer: Some(self.approvals_reviewer),
approval_policy: Some(self.step_settings.approval_policy.value()),
approvals_reviewer: Some(self.step_settings.approvals_reviewer),
permission_profile: Some(self.permission_profile()),
active_permission_profile: self.active_permission_profile(),
summary: self.model_reasoning_summary,
service_tier: Some(self.service_tier.clone()),
collaboration_mode: Some(self.collaboration_mode.clone()),
personality: self.personality,
summary: self.step_settings.reasoning_summary,
service_tier: Some(self.step_settings.service_tier.clone()),
collaboration_mode: Some(self.step_settings.collaboration_mode.clone()),
personality: self.step_settings.personality,
// TODO(anp): Include the runtime Windows sandbox override in restored settings.
// This omission predates TurnEnvironment::sandbox_context; rebuilding FromThread
// environments falls back to Config instead of retaining that override.
@@ -311,34 +308,15 @@ impl SessionConfiguration {
&self,
environments: &[TurnEnvironmentSelection],
) -> ConstraintResult<()> {
self.validate_auto_review_requirement(environments)?;
super::environment::validate_environment_selections(environments).map_err(|error| {
ConstraintError::InvalidValue {
field_name: "environments",
candidate: "environment configuration".to_string(),
allowed: format!("valid environment configuration ({error})"),
requirement_source: codex_config::RequirementSource::Unknown,
}
})
self.step_settings
.validate(&self.step_settings_constraints(environments))?;
super::environment::validate_environment_selections(environments)
}
fn validate_auto_review_requirement(
pub(super) fn step_settings_constraints(
&self,
environments: &[TurnEnvironmentSelection],
) -> ConstraintResult<()> {
if self.trusted_guardian_reviewer {
return Ok(());
}
let requirements = self
.original_config_do_not_use
.config_layer_stack
.requirements();
let model = self.collaboration_mode.model();
if !requirements.auto_review_required_for_model(model) {
return Ok(());
}
) -> StepSettingsConstraints<'_> {
let permission_profile = ThreadEnvironments::primary_config_for(environments)
.map(|config| config.permission_profile.permission_profile())
.unwrap_or_else(|| self.permission_profile_state.permission_profile())
@@ -346,21 +324,20 @@ impl SessionConfiguration {
.materialize_project_roots_with_workspace_roots(
&ThreadEnvironments::primary_workspace_roots_for(environments),
);
if self.approvals_reviewer == ApprovalsReviewer::AutoReview
&& !permission_profile
.file_system_sandbox_policy()
.has_full_disk_write_access()
&& self
StepSettingsConstraints {
requirements: self
.original_config_do_not_use
.config_layer_stack
.requirements(),
guardian_approval_enabled: self
.original_config_do_not_use
.features
.enabled(Feature::GuardianApproval)
{
return Ok(());
.enabled(Feature::GuardianApproval),
trusted_guardian_reviewer: self.trusted_guardian_reviewer,
has_full_disk_write_access: permission_profile
.file_system_sandbox_policy()
.has_full_disk_write_access(),
}
Err(ConstraintError::AutoReviewRequired {
model: model.to_string(),
})
}
pub(super) fn apply(
@@ -386,57 +363,6 @@ impl SessionConfiguration {
}
)
});
if let Some(collaboration_mode) = updates.collaboration_mode.clone() {
next_configuration.collaboration_mode = collaboration_mode;
}
if let Some(summary) = updates.reasoning_summary {
next_configuration.model_reasoning_summary = Some(summary);
}
if let Some(service_tier) = updates.service_tier.clone() {
// TODO(aibrahim): Remove once v2 clients no longer send the legacy
// "fast" service tier value.
next_configuration.service_tier = match service_tier {
Some(service_tier) => Some(
ServiceTier::from_request_value(&service_tier)
.map_or(service_tier, |service_tier| {
service_tier.request_value().to_string()
}),
),
None => Some(SERVICE_TIER_DEFAULT_REQUEST_VALUE.to_string()),
};
}
if let Some(personality) = updates.personality {
next_configuration.personality = Some(personality);
}
if let Some(approval_policy) = updates.approval_policy {
next_configuration.approval_policy.set(approval_policy)?;
}
if let Some(approvals_reviewer) = updates.approvals_reviewer {
next_configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.approvals_reviewer
.can_set(&approvals_reviewer)?;
next_configuration.approvals_reviewer = approvals_reviewer;
}
if !next_configuration.trusted_guardian_reviewer
&& self.collaboration_mode.model() != next_configuration.collaboration_mode.model()
&& next_configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.auto_review_required_for_model(next_configuration.collaboration_mode.model())
&& updates.approvals_reviewer.is_none()
{
next_configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.approvals_reviewer
.can_set(&ApprovalsReviewer::AutoReview)?;
next_configuration.approvals_reviewer = ApprovalsReviewer::AutoReview;
}
if let Some(windows_sandbox_level) = updates.windows_sandbox_level {
next_configuration.windows_sandbox_level = windows_sandbox_level;
}
@@ -548,7 +474,13 @@ impl SessionConfiguration {
.map_or(current_environments, |environments| {
environments.environments.as_slice()
});
next_configuration.validate(next_environments)?;
super::environment::validate_environment_selections(next_environments)?;
// Apply step settings last: the proposed permissions and environment
// selections must be complete before deriving their validation constraints.
next_configuration.step_settings = Arc::new(self.step_settings.apply(
&updates.step_settings,
&next_configuration.step_settings_constraints(next_environments),
)?);
Ok(next_configuration)
}
@@ -597,20 +529,15 @@ pub(crate) struct SessionSettingsCommit {
#[derive(Default, Clone)]
pub(crate) struct SessionSettingsUpdate {
pub(crate) step_settings: StepSettingsUpdate,
pub(crate) environments: Option<TurnEnvironmentSelections>,
pub(crate) profile_workspace_roots: Option<Vec<AbsolutePathBuf>>,
pub(crate) approval_policy: Option<AskForApproval>,
pub(crate) approvals_reviewer: Option<ApprovalsReviewer>,
pub(crate) sandbox_policy: Option<SandboxPolicy>,
pub(crate) permission_profile: Option<PermissionProfile>,
pub(crate) active_permission_profile: Option<ActivePermissionProfile>,
pub(crate) windows_sandbox_level: Option<WindowsSandboxLevel>,
pub(crate) collaboration_mode: Option<CollaborationMode>,
pub(crate) reasoning_summary: Option<ReasoningSummaryConfig>,
pub(crate) service_tier: Option<Option<String>>,
pub(crate) service_tier_for_turn: Option<String>,
pub(crate) final_output_json_schema: Option<Option<Value>>,
pub(crate) personality: Option<Personality>,
pub(crate) app_server_client_name: Option<String>,
pub(crate) app_server_client_version: Option<String>,
}
@@ -719,7 +646,10 @@ impl Session {
) -> anyhow::Result<Arc<Self>> {
debug!(
"Configuring session: model={}; provider={:?}",
session_configuration.collaboration_mode.model(),
session_configuration
.step_settings
.collaboration_mode
.model(),
session_configuration.provider
);
let base_instructions_provenance = if config.base_instructions.is_some() {
@@ -1031,9 +961,9 @@ impl Session {
session_source: session_configuration.session_source.clone(),
cwd: session_configuration.cwd().to_path_buf(),
rollout_path: rollout_path.clone(),
model: session_configuration.collaboration_mode.model().to_string(),
model: session_configuration.step_settings.collaboration_mode.model().to_string(),
provider_name: config.model_provider_id.clone(),
approval_policy: session_configuration.approval_policy.value().to_string(),
approval_policy: session_configuration.step_settings.approval_policy.value().to_string(),
sandbox_policy: format!(
"{:?}",
session_configuration.sandbox_policy(environment_selections)
@@ -1091,7 +1021,7 @@ impl Session {
let account_email = telemetry_auth.and_then(CodexAuth::get_account_email);
let originator = session_configuration.originator.clone();
let terminal_type = user_agent();
let session_model = session_configuration.collaboration_mode.model().to_string();
let session_model = session_configuration.step_settings.collaboration_mode.model().to_string();
let auth_env_telemetry = collect_auth_env_telemetry(
session_configuration.provider.info(),
auth_manager.codex_api_key_env_enabled(),
@@ -1148,7 +1078,7 @@ impl Session {
.collect::<Vec<_>>();
session_telemetry.conversation_starts(
config.model_provider.name.as_str(),
session_configuration.collaboration_mode.reasoning_effort(),
session_configuration.step_settings.collaboration_mode.reasoning_effort(),
config
.model_reasoning_summary
.unwrap_or(ReasoningSummaryConfig::Auto),
codex-rs/core/src/session/step_settings.rs
+174
View File
@@ -0,0 +1,174 @@
//! Configured inputs used to derive effective step settings.
use crate::config::Constrained;
use crate::config::ConstraintError;
use crate::config::ConstraintResult;
use codex_config::ConfigRequirements;
use codex_models_manager::ModelsManagerConfig;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::protocol::AskForApproval;
/// Model and execution settings selected for an individual model step within
/// a turn. A turn may contain several steps, each using its own captured settings.
#[derive(Clone, Debug, PartialEq)]
pub(crate) struct StepSettings {
pub(crate) collaboration_mode: CollaborationMode,
pub(crate) reasoning_summary: Option<ReasoningSummary>,
pub(crate) service_tier: Option<String>,
pub(crate) personality: Option<Personality>,
pub(crate) approval_policy: Constrained<AskForApproval>,
pub(crate) approvals_reviewer: ApprovalsReviewer,
}
/// Explicit startup overrides applied to catalog-derived model metadata.
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) struct ModelInfoOverrides {
pub(crate) context_window: Option<i64>,
pub(crate) auto_compact_token_limit: Option<i64>,
pub(crate) tool_output_token_limit: Option<usize>,
pub(crate) base_instructions: Option<String>,
}
impl From<ModelsManagerConfig> for ModelInfoOverrides {
fn from(config: ModelsManagerConfig) -> Self {
Self {
context_window: config.model_context_window,
auto_compact_token_limit: config.model_auto_compact_token_limit,
tool_output_token_limit: config.tool_output_token_limit,
base_instructions: config.base_instructions,
}
}
}
impl ModelInfoOverrides {
pub(crate) fn models_manager_config(
&self,
personality: Option<Personality>,
personality_enabled: bool,
) -> ModelsManagerConfig {
ModelsManagerConfig {
model_context_window: self.context_window,
model_auto_compact_token_limit: self.auto_compact_token_limit,
tool_output_token_limit: self.tool_output_token_limit,
base_instructions: self.base_instructions.clone(),
personality,
personality_enabled,
// The models manager already owns its catalog.
model_catalog: None,
}
}
}
/// Partial edits to configured step settings.
#[derive(Clone, Debug, Default, PartialEq)]
pub(crate) struct StepSettingsUpdate {
pub(crate) collaboration_mode: Option<CollaborationMode>,
pub(crate) reasoning_summary: Option<ReasoningSummary>,
pub(crate) service_tier: Option<Option<String>>,
pub(crate) personality: Option<Personality>,
pub(crate) approval_policy: Option<AskForApproval>,
pub(crate) approvals_reviewer: Option<ApprovalsReviewer>,
}
/// Constraints used when applying and validating a candidate settings version.
pub(crate) struct StepSettingsConstraints<'a> {
pub(crate) requirements: &'a ConfigRequirements,
pub(crate) guardian_approval_enabled: bool,
pub(crate) trusted_guardian_reviewer: bool,
pub(crate) has_full_disk_write_access: bool,
}
impl StepSettings {
/// Applies edits and validates the result against the supplied constraints.
/// Callers must supply the constraints of the proposed target environment.
pub(crate) fn apply(
&self,
update: &StepSettingsUpdate,
constraints: &StepSettingsConstraints<'_>,
) -> ConstraintResult<Self> {
let mut next = self.clone();
if let Some(collaboration_mode) = update.collaboration_mode.clone() {
next.collaboration_mode = collaboration_mode;
}
if let Some(summary) = update.reasoning_summary {
next.reasoning_summary = Some(summary);
}
if let Some(service_tier) = update.service_tier.clone() {
// TODO(aibrahim): Remove once v2 clients no longer send the legacy
// "fast" service tier value.
next.service_tier = Some(match service_tier {
Some(service_tier) => ServiceTier::from_request_value(&service_tier)
.map_or(service_tier, |service_tier| {
service_tier.request_value().to_string()
}),
None => SERVICE_TIER_DEFAULT_REQUEST_VALUE.to_string(),
});
}
if let Some(personality) = update.personality {
next.personality = Some(personality);
}
if let Some(approval_policy) = update.approval_policy {
next.approval_policy.set(approval_policy)?;
}
if let Some(approvals_reviewer) = update.approvals_reviewer {
constraints
.requirements
.approvals_reviewer
.can_set(&approvals_reviewer)?;
next.approvals_reviewer = approvals_reviewer;
}
if !constraints.trusted_guardian_reviewer
&& self.collaboration_mode.model() != next.collaboration_mode.model()
&& constraints
.requirements
.auto_review_required_for_model(next.collaboration_mode.model())
&& update.approvals_reviewer.is_none()
{
constraints
.requirements
.approvals_reviewer
.can_set(&ApprovalsReviewer::AutoReview)?;
next.approvals_reviewer = ApprovalsReviewer::AutoReview;
}
next.validate(constraints)?;
Ok(next)
}
/// Checks directly constructed settings or rechecks changed constraints.
pub(super) fn validate(
&self,
constraints: &StepSettingsConstraints<'_>,
) -> ConstraintResult<()> {
if constraints.trusted_guardian_reviewer {
return Ok(());
}
let model = self.collaboration_mode.model();
if !constraints
.requirements
.auto_review_required_for_model(model)
{
return Ok(());
}
if self.approvals_reviewer == ApprovalsReviewer::AutoReview
&& !constraints.has_full_disk_write_access
&& constraints.guardian_approval_enabled
{
return Ok(());
}
Err(ConstraintError::AutoReviewRequired {
model: model.to_string(),
})
}
}
#[cfg(test)]
#[path = "step_settings_tests.rs"]
mod tests;
codex-rs/core/src/session/step_settings_tests.rs
+272
View File
@@ -0,0 +1,272 @@
use super::*;
use crate::config::PermissionProfileState;
use crate::session::session::SessionConfiguration;
use crate::session::session::SessionSettingsUpdate;
use crate::session::tests::make_session_configuration_for_tests;
use codex_config::ConfigLayerStack;
use codex_config::RequirementSource;
use codex_config::Sourced;
use codex_features::Feature;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::TurnEnvironmentSelections;
use core_test_support::test_codex::local;
use pretty_assertions::assert_eq;
use std::collections::BTreeSet;
use std::sync::Arc;
fn set_requirements(configuration: &mut SessionConfiguration, requirements: ConfigRequirements) {
let config = Arc::make_mut(&mut configuration.original_config_do_not_use);
config.config_layer_stack = ConfigLayerStack::new(
config
.config_layer_stack
.all_layers_low_to_high()
.cloned()
.collect(),
requirements,
config.config_layer_stack.requirements_toml().clone(),
)
.expect("replace test requirements");
}
#[tokio::test]
async fn proposed_permission_profile_is_checked_before_step_settings() {
let mut configuration = make_session_configuration_for_tests().await;
let permission = Constrained::allow_only(PermissionProfile::read_only());
let permission_error = permission
.can_set(&PermissionProfile::Disabled)
.unwrap_err();
configuration.permission_profile_state =
PermissionProfileState::from_constrained_legacy(permission).unwrap();
let approval = Constrained::allow_only(AskForApproval::OnRequest);
let approval_error = approval.can_set(&AskForApproval::Never).unwrap_err();
Arc::make_mut(&mut configuration.step_settings).approval_policy = approval;
let mut requirements = configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.clone();
requirements.approvals_reviewer.value = Constrained::allow_only(ApprovalsReviewer::User);
let reviewer_error = requirements
.approvals_reviewer
.can_set(&ApprovalsReviewer::AutoReview)
.unwrap_err();
requirements.auto_review_required_models = Some(Sourced::new(
BTreeSet::from(["protected-model".to_string()]),
RequirementSource::Unknown,
));
set_requirements(&mut configuration, requirements);
let protected_mode = configuration.step_settings.collaboration_mode.with_updates(
Some("protected-model".to_string()),
/*effort*/ None,
/*developer_instructions*/ None,
);
let invalid_profile = SessionSettingsUpdate {
permission_profile: Some(PermissionProfile::Disabled),
..Default::default()
};
assert_eq!(
configuration.apply(&invalid_profile, &[]).err().as_ref(),
Some(&permission_error)
);
for (step_settings, expected) in [
(
StepSettingsUpdate {
approval_policy: Some(AskForApproval::Never),
..Default::default()
},
&approval_error,
),
(
StepSettingsUpdate {
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
..Default::default()
},
&reviewer_error,
),
(
StepSettingsUpdate {
collaboration_mode: Some(protected_mode),
..Default::default()
},
&reviewer_error,
),
] {
assert_eq!(
configuration
.step_settings
.apply(
&step_settings,
&configuration.step_settings_constraints(&[]),
)
.err()
.as_ref(),
Some(expected),
);
assert_eq!(
configuration
.apply(
&SessionSettingsUpdate {
step_settings,
..invalid_profile.clone()
},
&[]
)
.err()
.as_ref(),
Some(&permission_error),
);
}
}
#[tokio::test]
async fn model_review_requirement_uses_the_proposed_permission_profile() {
let mut configuration = make_session_configuration_for_tests().await;
configuration.permission_profile_state = PermissionProfileState::from_constrained_legacy(
Constrained::allow_any(PermissionProfile::Disabled),
)
.unwrap();
Arc::make_mut(&mut configuration.step_settings).approvals_reviewer = ApprovalsReviewer::User;
Arc::make_mut(&mut configuration.original_config_do_not_use)
.features
.enable(Feature::GuardianApproval)
.unwrap();
let mut requirements = configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.clone();
requirements.auto_review_required_models = Some(Sourced::new(
BTreeSet::from(["protected-model".to_string()]),
RequirementSource::Unknown,
));
set_requirements(&mut configuration, requirements);
let collaboration_mode = configuration.step_settings.collaboration_mode.with_updates(
Some("protected-model".to_string()),
/*effort*/ None,
/*developer_instructions*/ None,
);
let updates = SessionSettingsUpdate {
step_settings: StepSettingsUpdate {
collaboration_mode: Some(collaboration_mode.clone()),
..Default::default()
},
..Default::default()
};
assert_eq!(
configuration.apply(&updates, &[]).err(),
Some(ConstraintError::AutoReviewRequired {
model: "protected-model".to_string()
}),
);
let updated = configuration
.apply(
&SessionSettingsUpdate {
permission_profile: Some(PermissionProfile::read_only()),
..updates
},
&[],
)
.expect("the proposed restricted profile permits the protected model");
assert_eq!(
updated.step_settings.as_ref(),
&StepSettings {
collaboration_mode,
approvals_reviewer: ApprovalsReviewer::AutoReview,
..configuration.step_settings.as_ref().clone()
}
);
assert_eq!(updated.permission_profile(), PermissionProfile::read_only());
assert_eq!(
updated
.apply(
&SessionSettingsUpdate {
permission_profile: Some(PermissionProfile::Disabled),
..Default::default()
},
&[],
)
.err(),
Some(ConstraintError::AutoReviewRequired {
model: "protected-model".to_string(),
}),
);
}
#[tokio::test]
async fn environment_only_update_revalidates_existing_step_settings() {
let mut configuration = make_session_configuration_for_tests().await;
let profile = PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy {
kind: FileSystemSandboxKind::Restricted,
glob_scan_max_depth: None,
entries: vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
FileSystemAccessMode::Write,
),
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
FileSystemAccessMode::Read,
),
],
},
NetworkSandboxPolicy::Restricted,
);
configuration.permission_profile_state =
PermissionProfileState::from_constrained_legacy(Constrained::allow_any(profile)).unwrap();
let settings = Arc::make_mut(&mut configuration.step_settings);
settings.collaboration_mode = settings.collaboration_mode.with_updates(
Some("protected-model".to_string()),
/*effort*/ None,
/*developer_instructions*/ None,
);
settings.approvals_reviewer = ApprovalsReviewer::AutoReview;
Arc::make_mut(&mut configuration.original_config_do_not_use)
.features
.enable(Feature::GuardianApproval)
.unwrap();
let mut requirements = configuration
.original_config_do_not_use
.config_layer_stack
.requirements()
.clone();
requirements.auto_review_required_models = Some(Sourced::new(
BTreeSet::from(["protected-model".to_string()]),
RequirementSource::Unknown,
));
set_requirements(&mut configuration, requirements);
let environments = vec![local(configuration.cwd().clone())];
assert_eq!(configuration.validate(&environments), Ok(()));
// Removing the selected workspace also removes its read-only carveout,
// leaving this profile with full-disk write access.
assert_eq!(
configuration
.apply(
&SessionSettingsUpdate {
environments: Some(TurnEnvironmentSelections::new(
configuration.cwd().clone(),
Vec::new(),
)),
..Default::default()
},
&environments,
)
.err(),
Some(ConstraintError::AutoReviewRequired {
model: "protected-model".to_string(),
}),
);
}
codex-rs/core/src/session/tests.rs
+113 -60
View File
@@ -1,4 +1,6 @@
use super::mcp_refresh::McpRefresh;
use super::step_settings::StepSettings;
use super::step_settings::StepSettingsUpdate;
use super::turn_context::TurnEnvironment;
use super::*;
use crate::agents_md_manager::AgentsMdManager;
@@ -3290,7 +3292,10 @@ async fn config_change_contributor_observes_effective_config_changes() {
);
session
.update_settings(SessionSettingsUpdate {
collaboration_mode: Some(collaboration_mode),
step_settings: StepSettingsUpdate {
collaboration_mode: Some(collaboration_mode),
..Default::default()
},
..Default::default()
})
.await
@@ -4327,17 +4332,20 @@ async fn set_rate_limits_retains_previous_credits() {
};
let session_configuration = SessionConfiguration {
provider: create_model_provider(config.model_provider.clone(), /*auth_manager*/ None),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -4440,17 +4448,20 @@ async fn set_rate_limits_updates_plan_type_when_present() {
};
let session_configuration = SessionConfiguration {
provider: create_model_provider(config.model_provider.clone(), /*auth_manager*/ None),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -4946,7 +4957,10 @@ async fn session_settings_null_service_tier_update_uses_default_service_tier() {
let updated = session_configuration
.apply(
&SessionSettingsUpdate {
service_tier: Some(None),
step_settings: StepSettingsUpdate {
service_tier: Some(None),
..Default::default()
},
..Default::default()
},
&[],
@@ -4954,7 +4968,7 @@ async fn session_settings_null_service_tier_update_uses_default_service_tier() {
.expect("null service tier update should apply");
assert_eq!(
updated.service_tier,
updated.step_settings.service_tier,
Some(SERVICE_TIER_DEFAULT_REQUEST_VALUE.to_string())
);
}
@@ -4966,7 +4980,10 @@ async fn session_settings_legacy_fast_service_tier_update_uses_priority_request_
let updated = session_configuration
.apply(
&SessionSettingsUpdate {
service_tier: Some(Some("fast".to_string())),
step_settings: StepSettingsUpdate {
service_tier: Some(Some("fast".to_string())),
..Default::default()
},
..Default::default()
},
&[],
@@ -4974,7 +4991,7 @@ async fn session_settings_legacy_fast_service_tier_update_uses_priority_request_
.expect("legacy fast service tier update should apply");
assert_eq!(
updated.service_tier,
updated.step_settings.service_tier,
Some(ServiceTier::Fast.request_value().to_string())
);
}
@@ -4998,17 +5015,20 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
SessionConfiguration {
provider: create_model_provider(config.model_provider.clone(), /*auth_manager*/ None),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -5589,8 +5609,11 @@ async fn session_settings_commit_keeps_snapshot_across_postcommit_wait() {
.expect("network refresh lock");
let mut first_update = Box::pin(tokio::task::unconstrained(session.update_settings(
SessionSettingsUpdate {
step_settings: StepSettingsUpdate {
service_tier: Some(Some(ServiceTier::Fast.request_value().to_string())),
..Default::default()
},
permission_profile: Some(PermissionProfile::read_only()),
service_tier: Some(Some(ServiceTier::Fast.request_value().to_string())),
..Default::default()
},
)));
@@ -5603,7 +5626,10 @@ async fn session_settings_commit_keeps_snapshot_across_postcommit_wait() {
let expected = session.thread_settings_snapshot().await;
let later_commit = session
.update_settings(SessionSettingsUpdate {
service_tier: Some(None),
step_settings: StepSettingsUpdate {
service_tier: Some(None),
..Default::default()
},
..Default::default()
})
.await
@@ -5838,17 +5864,20 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
config.model_provider.clone(),
Some(Arc::clone(&auth_manager)),
),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -5988,17 +6017,20 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
config.model_provider.clone(),
Some(Arc::clone(&auth_manager)),
),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -6278,17 +6310,20 @@ async fn make_session_with_config_and_rx(
config.model_provider.clone(),
Some(Arc::clone(&auth_manager)),
),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -6401,17 +6436,20 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
config.model_provider.clone(),
Some(Arc::clone(&auth_manager)),
),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -8205,17 +8243,20 @@ where
config.model_provider.clone(),
Some(Arc::clone(&auth_manager)),
),
collaboration_mode,
model_reasoning_summary: config.model_reasoning_summary,
step_settings: Arc::new(StepSettings {
collaboration_mode,
reasoning_summary: config.model_reasoning_summary,
service_tier: None,
personality: config.personality,
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
}),
model_info_overrides: config.to_models_manager_config().into(),
developer_instructions: config.developer_instructions.clone(),
service_tier: None,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
approval_policy: config.permissions.approval_policy.clone(),
approvals_reviewer: config.approvals_reviewer,
permission_profile_state: config.permissions.permission_profile_state().clone(),
allow_login_shell: config.permissions.allow_login_shell,
shell_environment_policy: config.permissions.shell_environment_policy.clone(),
@@ -8574,8 +8615,11 @@ async fn refreshed_mcp_binding_captures_current_approval_authority() {
session
.update_settings(SessionSettingsUpdate {
approval_policy: Some(AskForApproval::Never),
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
step_settings: StepSettingsUpdate {
approval_policy: Some(AskForApproval::Never),
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
..Default::default()
},
permission_profile: Some(PermissionProfile::Disabled),
..Default::default()
})
@@ -8645,7 +8689,10 @@ async fn mcp_elicitation_reviewer_uses_latest_runtime_authority() {
session
.update_settings(SessionSettingsUpdate {
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
step_settings: StepSettingsUpdate {
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
..Default::default()
},
..Default::default()
})
.await
@@ -8687,7 +8734,10 @@ async fn mcp_elicitation_reviewer_uses_latest_runtime_authority() {
session
.update_settings(SessionSettingsUpdate {
approval_policy: Some(AskForApproval::Never),
step_settings: StepSettingsUpdate {
approval_policy: Some(AskForApproval::Never),
..Default::default()
},
..Default::default()
})
.await
@@ -8781,7 +8831,10 @@ async fn mcp_policy_changes_schedule_runtime_refresh() {
.new_turn_with_sub_id(
"policy-change".to_string(),
SessionSettingsUpdate {
approval_policy: Some(AskForApproval::Never),
step_settings: StepSettingsUpdate {
approval_policy: Some(AskForApproval::Never),
..Default::default()
},
..Default::default()
},
)
codex-rs/core/src/session/thread_settings.rs
+10 -6
View File
@@ -3,6 +3,7 @@
use super::session::Session;
use super::session::SessionSettingsUpdate;
use super::step_settings::StepSettingsUpdate;
use crate::config::ConstraintResult;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::ErrorEvent;
@@ -63,23 +64,26 @@ pub(super) async fn prepare_update(
// partial thread-settings updates refresh those fields on the active mode.
state
.session_configuration
.step_settings
.collaboration_mode
.with_updates(model, effort, /*developer_instructions*/ None)
}
};
SessionSettingsUpdate {
step_settings: StepSettingsUpdate {
collaboration_mode: Some(collaboration_mode),
reasoning_summary: summary,
service_tier,
personality,
approval_policy,
approvals_reviewer,
},
environments,
profile_workspace_roots,
approval_policy,
approvals_reviewer,
sandbox_policy,
permission_profile,
active_permission_profile,
windows_sandbox_level,
collaboration_mode: Some(collaboration_mode),
reasoning_summary: summary,
service_tier,
personality,
..Default::default()
}
}
codex-rs/core/src/session/turn_context.rs
+37 -15
View File
@@ -572,18 +572,22 @@ impl Session {
let config = session_configuration.original_config_do_not_use.clone();
let mut per_turn_config = (*config).clone();
per_turn_config.cwd = cwd;
per_turn_config.permissions.approval_policy = session_configuration.approval_policy.clone();
per_turn_config.permissions.approval_policy =
session_configuration.step_settings.approval_policy.clone();
let workspace_roots = self.services.turn_environments.primary_workspace_roots();
per_turn_config.workspace_roots = workspace_roots.clone();
per_turn_config
.permissions
.set_workspace_roots(workspace_roots);
per_turn_config.model_reasoning_effort =
session_configuration.collaboration_mode.reasoning_effort();
per_turn_config.model_reasoning_summary = session_configuration.model_reasoning_summary;
per_turn_config.service_tier = session_configuration.service_tier.clone();
per_turn_config.personality = session_configuration.personality;
per_turn_config.approvals_reviewer = session_configuration.approvals_reviewer;
per_turn_config.model_reasoning_effort = session_configuration
.step_settings
.collaboration_mode
.reasoning_effort();
per_turn_config.model_reasoning_summary =
session_configuration.step_settings.reasoning_summary;
per_turn_config.service_tier = session_configuration.step_settings.service_tier.clone();
per_turn_config.personality = session_configuration.step_settings.personality;
per_turn_config.approvals_reviewer = session_configuration.step_settings.approvals_reviewer;
session_configuration
.apply_permission_profile_to_permissions(&mut per_turn_config.permissions);
let permission_profile = session_configuration.permission_profile();
@@ -614,7 +618,13 @@ impl Session {
) -> Config {
let mut config =
self.build_per_turn_config(session_configuration, session_configuration.cwd().clone());
config.model = Some(session_configuration.collaboration_mode.model().to_string());
config.model = Some(
session_configuration
.step_settings
.collaboration_mode
.model()
.to_string(),
);
config
}
@@ -640,13 +650,17 @@ impl Session {
sub_id: String,
skills_snapshot: HostSkillsSnapshot,
) -> TurnContext {
let collaboration_mode = &session_configuration.collaboration_mode;
let collaboration_mode = &session_configuration.step_settings.collaboration_mode;
let reasoning_effort = collaboration_mode.reasoning_effort();
let reasoning_summary = session_configuration
.model_reasoning_summary
.step_settings
.reasoning_summary
.unwrap_or(model_info.default_reasoning_summary);
let session_telemetry = session_telemetry.clone().with_model(
session_configuration.collaboration_mode.model(),
session_configuration
.step_settings
.collaboration_mode
.model(),
model_info.slug.as_str(),
);
let session_source = session_configuration.session_source.clone();
@@ -723,7 +737,7 @@ impl Session {
.developer_instructions
.clone(),
multi_agent_version,
personality: session_configuration.personality,
personality: session_configuration.step_settings.personality,
network,
windows_sandbox_level: session_configuration.windows_sandbox_level,
available_models,
@@ -768,7 +782,7 @@ impl Session {
let mut configuration = commit.configuration;
// Apply the override only to the turn's copy, after persisting thread settings.
if let Some(service_tier) = service_tier_for_turn {
configuration.service_tier = Some(service_tier);
Arc::make_mut(&mut configuration.step_settings).service_tier = Some(service_tier);
}
let turn_context = self
.new_turn_from_configuration(sub_id, configuration, final_output_json_schema)
@@ -833,8 +847,16 @@ impl Session {
.services
.models_manager
.get_model_info(
session_configuration.collaboration_mode.model(),
&per_turn_config.to_models_manager_config(),
session_configuration
.step_settings
.collaboration_mode
.model(),
&session_configuration
.model_info_overrides
.models_manager_config(
session_configuration.step_settings.personality,
self.features.enabled(Feature::Personality),
),
)
.await;
self.services
codex-rs/core/src/session/turn_input.rs
+1 -1
View File
@@ -85,7 +85,7 @@ impl PreparedTurnInputSettings {
fn would_enter_plan_mode(&self) -> bool {
self.thread_settings_update
.as_ref()
.and_then(|updates| updates.collaboration_mode.as_ref())
.and_then(|updates| updates.step_settings.collaboration_mode.as_ref())
.is_some_and(|collaboration_mode| collaboration_mode.mode == ModeKind::Plan)
}
codex-rs/core/src/session/turn_input_tests.rs
+7 -4
View File
@@ -133,7 +133,7 @@ async fn accepted_input_applies_thread_settings() {
let state = session.state.lock().await;
assert_eq!(
state.session_configuration.approvals_reviewer,
state.session_configuration.step_settings.approvals_reviewer,
codex_config::types::ApprovalsReviewer::AutoReview
);
assert!(
@@ -240,7 +240,8 @@ async fn start_only_rejects_plan_mode_without_injecting() {
collaboration_mode.mode = ModeKind::Plan;
{
let mut state = session.state.lock().await;
state.session_configuration.collaboration_mode = collaboration_mode;
Arc::make_mut(&mut state.session_configuration.step_settings).collaboration_mode =
collaboration_mode;
}
let submission = submit_start_only(
@@ -272,7 +273,8 @@ async fn start_only_accepts_user_input_in_plan_mode() {
collaboration_mode.mode = ModeKind::Plan;
{
let mut state = session.state.lock().await;
state.session_configuration.collaboration_mode = collaboration_mode;
Arc::make_mut(&mut state.session_configuration.step_settings).collaboration_mode =
collaboration_mode;
state.merge_connector_selection(["calendar".to_string()]);
}
@@ -307,7 +309,8 @@ async fn start_only_rejects_empty_user_input_in_plan_mode() {
collaboration_mode.mode = ModeKind::Plan;
{
let mut state = session.state.lock().await;
state.session_configuration.collaboration_mode = collaboration_mode;
Arc::make_mut(&mut state.session_configuration.step_settings).collaboration_mode =
collaboration_mode;
}
let submission = submit_start_only(
codex-rs/core/tests/suite/mod.rs
+1
View File
@@ -146,6 +146,7 @@ mod safety_check_downgrade;
mod search_tool;
mod send_user_message_async;
mod settings_commits;
mod settings_constraints;
mod shell_snapshot;
mod skill_approval;
mod skills;
codex-rs/core/tests/suite/settings_constraints.rs
+217
View File
@@ -0,0 +1,217 @@
use anyhow::Result;
use codex_config::test_support::CloudConfigBundleFixture;
use codex_core::CodexThread;
use codex_core::TurnInputRequest;
use codex_core::TurnInputSubmission;
use codex_core::config::Constrained;
use codex_features::Feature;
use codex_models_manager::model_info::model_info_from_slug;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ApprovalMessages;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::openai_models::PermissionMessages;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ThreadSettingsOverrides;
use codex_protocol::protocol::ThreadSettingsSnapshot;
use codex_protocol::user_input::UserInput;
use core_test_support::responses;
use core_test_support::skip_if_no_network;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use std::time::Duration;
use test_case::test_case;
use tokio::time::timeout;
const INITIAL_MODEL: &str = "gpt-5.4";
const PROTECTED_MODEL: &str = "gpt-5.2";
const REVIEW_INSTRUCTIONS: &str = "Protected-model automatic review instructions.";
const PERMISSION_INSTRUCTIONS: &str = "Protected-model read-only instructions.";
#[derive(Clone, Copy)]
enum SettingsOperation {
Standalone,
TurnStart,
}
impl SettingsOperation {
async fn submit(
self,
codex: &CodexThread,
thread_settings: ThreadSettingsOverrides,
) -> Result<ThreadSettingsSnapshot> {
let id = match self {
Self::Standalone => codex.submit(Op::ThreadSettings { thread_settings }).await?,
Self::TurnStart => {
let result = codex
.start_or_steer_turn(
TurnInputRequest::user_input(vec![UserInput::Text {
text: "use the protected model".to_string(),
text_elements: Vec::new(),
}])
.with_thread_settings(thread_settings),
)
.await?;
let TurnInputSubmission::Started { turn_id } = result else {
anyhow::bail!("expected a new turn, got {result:?}");
};
turn_id
}
};
timeout(Duration::from_secs(10), async {
loop {
let event = codex.next_event().await?;
if event.id != id {
continue;
}
match event.msg {
EventMsg::ThreadSettingsApplied(applied) => return Ok(applied.thread_settings),
EventMsg::Error(error) => {
assert_eq!(error.codex_error_info, Some(CodexErrorInfo::BadRequest));
anyhow::bail!(error.message);
}
_ => {}
}
}
})
.await?
}
}
#[test_case(SettingsOperation::Standalone; "standalone settings")]
#[test_case(SettingsOperation::TurnStart; "turn-start settings")]
#[tokio::test]
async fn protected_model_settings_use_the_proposed_permissions(
operation: SettingsOperation,
) -> Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let response = responses::mount_sse_once(
&server,
responses::sse(vec![
responses::ev_response_created("response"),
responses::ev_completed("response"),
]),
)
.await;
let mut protected = model_info_from_slug(PROTECTED_MODEL);
let messages = protected.model_messages.as_mut().expect("model messages");
messages.approvals = Some(ApprovalMessages {
on_request: None,
on_request_auto_review: Some(REVIEW_INSTRUCTIONS.to_string()),
never: None,
unless_trusted: None,
});
messages.permissions = Some(PermissionMessages {
danger_full_access: None,
workspace_write: None,
read_only: Some(PERMISSION_INSTRUCTIONS.to_string()),
});
let test = test_codex()
.with_model(INITIAL_MODEL)
.with_cloud_config_bundle(
CloudConfigBundleFixture::loader_with_enterprise_requirement(
"[auto_review]\nrequired_on_models = [\"gpt-5.2\"]\n",
),
)
.with_config(move |config| {
config.model_catalog = Some(ModelsResponse {
models: vec![model_info_from_slug(INITIAL_MODEL), protected],
});
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config.approvals_reviewer = ApprovalsReviewer::User;
config
.permissions
.set_permission_profile(PermissionProfile::Disabled)
.expect("full-access profile should be allowed");
config
.features
.enable(Feature::GuardianApproval)
.expect("test config should allow Guardian");
})
.build_with_auto_env(&server)
.await?;
let initial = test.codex.thread_settings_snapshot().await;
assert_eq!(initial.permission_profile, PermissionProfile::Disabled);
let model_update = ThreadSettingsOverrides {
model: Some(PROTECTED_MODEL.to_string()),
..Default::default()
};
let error = operation
.submit(&test.codex, model_update.clone())
.await
.expect_err("the protected model requires restricted permissions");
assert!(
error.to_string().contains("you need to use auto review"),
"{error}"
);
assert_eq!(test.codex.thread_settings_snapshot().await, initial);
assert!(response.requests().is_empty());
let expected = ThreadSettingsSnapshot {
model: PROTECTED_MODEL.to_string(),
collaboration_mode: initial.collaboration_mode.with_updates(
Some(PROTECTED_MODEL.to_string()),
/*effort*/ None,
/*developer_instructions*/ None,
),
approvals_reviewer: ApprovalsReviewer::AutoReview,
permission_profile: PermissionProfile::read_only(),
active_permission_profile: None,
..initial
};
let applied = operation
.submit(
&test.codex,
ThreadSettingsOverrides {
permission_profile: Some(PermissionProfile::read_only()),
..model_update
},
)
.await?;
assert_eq!(applied, expected);
assert_eq!(test.codex.thread_settings_snapshot().await, expected);
if let SettingsOperation::TurnStart = operation {
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
}
// Even an update with no step-settings edits must revalidate the existing
// protected model against the proposed permissions.
let error = operation
.submit(
&test.codex,
ThreadSettingsOverrides {
permission_profile: Some(PermissionProfile::Disabled),
..Default::default()
},
)
.await
.expect_err("full access must invalidate the protected model settings");
assert!(
error.to_string().contains("you need to use auto review"),
"{error}"
);
assert_eq!(test.codex.thread_settings_snapshot().await, expected);
if let SettingsOperation::Standalone = operation {
test.submit_text_turn("use the committed settings").await?;
}
let request = response.single_request();
assert_eq!(request.body_json()["model"], PROTECTED_MODEL);
let instructions = request.message_input_texts("developer").join("\n");
assert!(instructions.contains(REVIEW_INSTRUCTIONS), "{instructions}");
assert!(
instructions.contains(PERMISSION_INSTRUCTIONS),
"{instructions}"
);
Ok(())
}