Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion codex-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 1 addition & 12 deletions codex-rs/core/src/guardian/review_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,17 +703,6 @@ async fn run_review_on_session(
.unwrap_or_default();

let permission_profile = PermissionProfile::read_only();
let sandbox_policy =
match permission_profile.to_legacy_sandbox_policy(params.parent_turn.cwd.as_path()) {
Ok(sandbox_policy) => sandbox_policy,
Err(err) => {
return (
GuardianReviewSessionOutcome::SessionFailed(err.into()),
false,
analytics_result,
);
}
};

let submit_result = run_before_review_deadline(
deadline,
Expand All @@ -724,7 +713,7 @@ async fn run_review_on_session(
cwd: params.parent_turn.cwd.to_path_buf(),
approval_policy: AskForApproval::Never,
approvals_reviewer: None,
sandbox_policy,
sandbox_policy: None,
permission_profile: Some(permission_profile),
model: params.model.clone(),
effort: params.reasoning_effort,
Expand Down
5 changes: 3 additions & 2 deletions codex-rs/core/src/session/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,11 @@ pub(super) async fn user_input_or_turn_inner(
},
})
});
let clear_active_permission_profile = permission_profile.is_none();
let clear_active_permission_profile =
permission_profile.is_none() && sandbox_policy.is_some();
let permission_profile = permission_profile_with_legacy_fallback(
sess,
Some(&sandbox_policy),
sandbox_policy.as_ref(),
permission_profile,
Some(cwd.as_path()),
)
Expand Down
4 changes: 2 additions & 2 deletions codex-rs/core/src/session/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4257,8 +4257,8 @@ async fn user_turn_updates_approvals_reviewer() {
cwd: config.cwd.to_path_buf(),
approval_policy: config.permissions.approval_policy.value(),
approvals_reviewer: Some(codex_config::types::ApprovalsReviewer::AutoReview),
sandbox_policy: config.legacy_sandbox_policy(),
permission_profile: None,
sandbox_policy: None,
permission_profile: Some(config.permissions.permission_profile()),
model: turn_context.model_info.slug.clone(),
effort: config.model_reasoning_effort,
summary: config.model_reasoning_summary,
Expand Down
1 change: 0 additions & 1 deletion codex-rs/core/tests/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ codex-login = { workspace = true }
codex-model-provider-info = { workspace = true }
codex-models-manager = { workspace = true }
codex-protocol = { workspace = true }
codex-sandboxing = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-cargo-bin = { workspace = true }
ctor = { workspace = true }
Expand Down
21 changes: 9 additions & 12 deletions codex-rs/core/tests/common/test_codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@ use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::RealtimeConversationVersion as RealtimeWsVersion;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionConfiguredEvent;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::user_input::UserInput;
use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile;
use codex_utils_absolute_path::AbsolutePathBuf;
use futures::future::BoxFuture;
use serde_json::Value;
Expand Down Expand Up @@ -206,18 +204,17 @@ pub enum ShellModelOutput {

/// Returns the permission fields required by `Op::UserTurn` for tests that
/// construct the op directly.
///
/// The legacy sandbox field is intentionally omitted when a canonical
/// `PermissionProfile` is available.
pub fn turn_permission_fields(
permission_profile: PermissionProfile,
cwd: &Path,
) -> (SandboxPolicy, Option<PermissionProfile>) {
let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy();
let sandbox_policy = compatibility_sandbox_policy_for_permission_profile(
&permission_profile,
&file_system_sandbox_policy,
permission_profile.network_sandbox_policy(),
cwd,
);
(sandbox_policy, Some(permission_profile))
_cwd: &Path,
) -> (
Option<codex_protocol::protocol::SandboxPolicy>,
Option<PermissionProfile>,
) {
(None, Some(permission_profile))
}

pub struct TestCodexBuilder {
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/tests/suite/prompt_caching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an
cwd: None,
approval_policy: Some(AskForApproval::Never),
approvals_reviewer: None,
sandbox_policy: Some(sandbox_policy),
sandbox_policy,
permission_profile,
windows_sandbox_level: None,
model: None,
Expand Down
11 changes: 8 additions & 3 deletions codex-rs/protocol/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ pub enum Op {
/// User input items, see `InputItem`
items: Vec<UserInput>,

/// `cwd` to use with the [`SandboxPolicy`] and potentially tool calls
/// `cwd` to use with the permissions profile and potentially tool calls
/// such as `local_shell`.
cwd: PathBuf,

Expand All @@ -541,8 +541,13 @@ pub enum Op {
/// When omitted, the session keeps the current setting
approvals_reviewer: Option<ApprovalsReviewer>,

/// Policy to use for tool calls such as `local_shell`.
sandbox_policy: SandboxPolicy,
/// Legacy sandbox policy to use for tool calls such as `local_shell`.
///
/// When omitted, `permission_profile` is used as the canonical source
/// of permissions. This field is kept only as a compatibility fallback
/// for older callers that have not migrated to `permission_profile`.
#[serde(default, skip_serializing_if = "Option::is_none")]
sandbox_policy: Option<SandboxPolicy>,

/// Full permissions profile to use for tool calls such as `local_shell`.
///
Expand Down
42 changes: 20 additions & 22 deletions codex-rs/tui/src/app_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,26 +370,22 @@ impl AppCommand {
final_output_json_schema,
collaboration_mode,
personality,
} => {
let sandbox_policy =
legacy_compatible_sandbox_policy(&permission_profile, cwd.as_path());
Op::UserTurn {
items,
environments: None,
cwd,
approval_policy,
approvals_reviewer,
sandbox_policy,
permission_profile: Some(permission_profile),
model,
effort,
summary,
service_tier,
final_output_json_schema,
collaboration_mode,
personality,
}
}
} => Op::UserTurn {
items,
environments: None,
cwd,
approval_policy,
approvals_reviewer,
sandbox_policy: None,
permission_profile: Some(permission_profile),
model,
effort,
summary,
service_tier,
final_output_json_schema,
collaboration_mode,
personality,
},
Self::OverrideTurnContext {
cwd,
approval_policy,
Expand Down Expand Up @@ -606,8 +602,10 @@ impl From<Op> for AppCommand {
personality,
} => {
if environments.is_none()
&& legacy_compatible_sandbox_policy(&permission_profile, cwd.as_path())
== sandbox_policy
&& sandbox_policy.as_ref().is_none_or(|sandbox_policy| {
legacy_compatible_sandbox_policy(&permission_profile, cwd.as_path())
== *sandbox_policy
})
{
Self::UserTurn {
items,
Expand Down
Loading