mirror of
https://github.com/openai/codex.git
synced 2026-05-24 21:14:51 +00:00
Move workspace roots onto thread/session state and stop using active permission profile modifications as an overlay for writable roots. Existing app-server threads now preserve their persisted PermissionProfile value across resume, fork, and turn updates; permissions requests on existing threads only update the active named profile after validating it exists. Workspace roots can be updated independently, and SandboxPolicy::WorkspaceWrite no longer stores its own writable_roots.
This commit is contained in:
@@ -24,7 +24,6 @@ use codex_app_server_protocol::ConfigWarningNotification;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_app_server_protocol::McpServerElicitationAction;
|
||||
use codex_app_server_protocol::McpServerElicitationRequestResponse;
|
||||
use codex_app_server_protocol::PermissionProfileModificationParams;
|
||||
use codex_app_server_protocol::PermissionProfileSelectionParams;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ReviewStartParams;
|
||||
@@ -81,7 +80,6 @@ use codex_protocol::SessionId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
use codex_protocol::models::ActivePermissionProfile;
|
||||
use codex_protocol::models::ActivePermissionProfileModification;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewRequest;
|
||||
@@ -422,6 +420,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
show_raw_agent_reasoning: oss.then_some(true),
|
||||
tools_web_search_request: None,
|
||||
ephemeral: ephemeral.then_some(true),
|
||||
workspace_roots: None,
|
||||
additional_writable_roots: add_dir,
|
||||
};
|
||||
|
||||
@@ -775,6 +774,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
|
||||
responsesapi_client_metadata: None,
|
||||
environments: None,
|
||||
cwd: Some(default_cwd),
|
||||
workspace_roots: None,
|
||||
approval_policy: Some(default_approval_policy.into()),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: None,
|
||||
@@ -946,6 +946,7 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams {
|
||||
model: config.model.clone(),
|
||||
model_provider: Some(config.model_provider_id.clone()),
|
||||
cwd: Some(config.cwd.to_string_lossy().to_string()),
|
||||
workspace_roots: Some(config.workspace_roots.clone()),
|
||||
approval_policy: Some(config.permissions.approval_policy.value().into()),
|
||||
approvals_reviewer: approvals_reviewer_override_from_config(config),
|
||||
sandbox: sandbox.flatten(),
|
||||
@@ -958,20 +959,15 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams {
|
||||
|
||||
fn thread_resume_params_from_config(config: &Config, thread_id: String) -> ThreadResumeParams {
|
||||
let permissions = permissions_selection_from_config(config);
|
||||
let sandbox = permissions.is_none().then(|| {
|
||||
sandbox_mode_from_permission_profile(
|
||||
&config.permissions.permission_profile(),
|
||||
config.cwd.as_path(),
|
||||
)
|
||||
});
|
||||
ThreadResumeParams {
|
||||
thread_id,
|
||||
model: config.model.clone(),
|
||||
model_provider: Some(config.model_provider_id.clone()),
|
||||
cwd: Some(config.cwd.to_string_lossy().to_string()),
|
||||
workspace_roots: Some(config.workspace_roots.clone()),
|
||||
approval_policy: Some(config.permissions.approval_policy.value().into()),
|
||||
approvals_reviewer: approvals_reviewer_override_from_config(config),
|
||||
sandbox: sandbox.flatten(),
|
||||
sandbox: None,
|
||||
permissions,
|
||||
config: config_request_overrides_from_config(config),
|
||||
..ThreadResumeParams::default()
|
||||
@@ -988,19 +984,7 @@ fn permissions_selection_from_config(config: &Config) -> Option<PermissionProfil
|
||||
fn permissions_selection_from_active_profile(
|
||||
active: ActivePermissionProfile,
|
||||
) -> PermissionProfileSelectionParams {
|
||||
let modifications = active
|
||||
.modifications
|
||||
.into_iter()
|
||||
.map(|modification| match modification {
|
||||
ActivePermissionProfileModification::AdditionalWritableRoot { path } => {
|
||||
PermissionProfileModificationParams::AdditionalWritableRoot { path }
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
PermissionProfileSelectionParams::Profile {
|
||||
id: active.id,
|
||||
modifications: (!modifications.is_empty()).then_some(modifications),
|
||||
}
|
||||
PermissionProfileSelectionParams::Profile { id: active.id }
|
||||
}
|
||||
|
||||
fn sandbox_mode_from_permission_profile(
|
||||
@@ -1079,6 +1063,7 @@ fn session_configured_from_thread_start_response(
|
||||
.unwrap_or_else(|| config.permissions.permission_profile()),
|
||||
response.active_permission_profile.clone().map(Into::into),
|
||||
response.cwd.clone(),
|
||||
response.workspace_roots.clone(),
|
||||
response.reasoning_effort,
|
||||
)
|
||||
}
|
||||
@@ -1104,6 +1089,7 @@ fn session_configured_from_thread_resume_response(
|
||||
.unwrap_or_else(|| config.permissions.permission_profile()),
|
||||
response.active_permission_profile.clone().map(Into::into),
|
||||
response.cwd.clone(),
|
||||
response.workspace_roots.clone(),
|
||||
response.reasoning_effort,
|
||||
)
|
||||
}
|
||||
@@ -1134,12 +1120,18 @@ fn session_configured_from_thread_response(
|
||||
permission_profile: PermissionProfile,
|
||||
active_permission_profile: Option<codex_protocol::models::ActivePermissionProfile>,
|
||||
cwd: AbsolutePathBuf,
|
||||
workspace_roots: Vec<AbsolutePathBuf>,
|
||||
reasoning_effort: Option<codex_protocol::openai_models::ReasoningEffort>,
|
||||
) -> Result<SessionConfiguredEvent, String> {
|
||||
let session_id = SessionId::from_string(session_id)
|
||||
.map_err(|err| format!("session id `{session_id}` is invalid: {err}"))?;
|
||||
let thread_id = ThreadId::from_string(thread_id)
|
||||
.map_err(|err| format!("thread id `{thread_id}` is invalid: {err}"))?;
|
||||
let workspace_roots = if workspace_roots.is_empty() {
|
||||
vec![cwd.clone()]
|
||||
} else {
|
||||
workspace_roots
|
||||
};
|
||||
|
||||
Ok(SessionConfiguredEvent {
|
||||
session_id,
|
||||
@@ -1155,6 +1147,7 @@ fn session_configured_from_thread_response(
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
reasoning_effort,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
|
||||
@@ -456,7 +456,7 @@ async fn thread_start_params_include_review_policy_when_auto_review_is_enabled()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_lifecycle_params_include_legacy_sandbox_when_no_active_profile() {
|
||||
async fn thread_lifecycle_params_handle_legacy_sandbox_when_no_active_profile() {
|
||||
let codex_home = tempdir().expect("create temp codex home");
|
||||
let cwd = tempdir().expect("create temp cwd");
|
||||
let config = ConfigBuilder::default()
|
||||
@@ -479,10 +479,7 @@ async fn thread_lifecycle_params_include_legacy_sandbox_when_no_active_profile()
|
||||
Some(codex_app_server_protocol::SandboxMode::DangerFullAccess)
|
||||
);
|
||||
assert_eq!(start_params.permissions, None);
|
||||
assert_eq!(
|
||||
resume_params.sandbox,
|
||||
Some(codex_app_server_protocol::SandboxMode::DangerFullAccess)
|
||||
);
|
||||
assert_eq!(resume_params.sandbox, None);
|
||||
assert_eq!(resume_params.permissions, None);
|
||||
}
|
||||
|
||||
@@ -531,6 +528,26 @@ async fn session_configured_from_thread_response_uses_permission_profile_from_re
|
||||
assert_eq!(event.permission_profile, PermissionProfile::Disabled);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_configured_from_thread_response_uses_workspace_roots_from_response() {
|
||||
let codex_home = tempdir().expect("create temp codex home");
|
||||
let cwd = tempdir().expect("create temp cwd");
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(cwd.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect("build config");
|
||||
let mut response = sample_thread_start_response();
|
||||
let extra_root = test_path_buf("/tmp/extra-root").abs();
|
||||
response.workspace_roots = vec![response.cwd.clone(), extra_root.clone()];
|
||||
|
||||
let event = session_configured_from_thread_start_response(&response, &config)
|
||||
.expect("build bootstrap session configured event");
|
||||
|
||||
assert_eq!(event.workspace_roots, vec![response.cwd, extra_root]);
|
||||
}
|
||||
|
||||
fn sample_thread_start_response() -> ThreadStartResponse {
|
||||
ThreadStartResponse {
|
||||
thread: codex_app_server_protocol::Thread {
|
||||
@@ -558,11 +575,11 @@ fn sample_thread_start_response() -> ThreadStartResponse {
|
||||
model_provider: "openai".to_string(),
|
||||
service_tier: None,
|
||||
cwd: test_path_buf("/tmp").abs(),
|
||||
workspace_roots: Vec::new(),
|
||||
instruction_sources: Vec::new(),
|
||||
approval_policy: codex_app_server_protocol::AskForApproval::OnRequest,
|
||||
approvals_reviewer: codex_app_server_protocol::ApprovalsReviewer::AutoReview,
|
||||
sandbox: codex_app_server_protocol::SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
|
||||
@@ -121,6 +121,7 @@ fn session_configured_produces_thread_started_event() {
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile: None,
|
||||
cwd: test_path_buf("/tmp/project").abs(),
|
||||
workspace_roots: Vec::new(),
|
||||
reasoning_effort: None,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
|
||||
@@ -16,6 +16,7 @@ async fn spawn_command_under_sandbox(
|
||||
command_cwd: AbsolutePathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
sandbox_cwd: &AbsolutePathBuf,
|
||||
additional_workspace_roots: &[AbsolutePathBuf],
|
||||
stdio_policy: StdioPolicy,
|
||||
env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child> {
|
||||
@@ -28,6 +29,8 @@ async fn spawn_command_under_sandbox(
|
||||
use std::process::Stdio;
|
||||
|
||||
let codex_linux_sandbox_exe = None;
|
||||
let mut workspace_roots = vec![sandbox_cwd.clone()];
|
||||
workspace_roots.extend_from_slice(additional_workspace_roots);
|
||||
let exec_request = build_exec_request(
|
||||
ExecParams {
|
||||
command,
|
||||
@@ -44,6 +47,7 @@ async fn spawn_command_under_sandbox(
|
||||
},
|
||||
&PermissionProfile::from_legacy_sandbox_policy(sandbox_policy),
|
||||
sandbox_cwd,
|
||||
&workspace_roots,
|
||||
&codex_linux_sandbox_exe,
|
||||
/*use_legacy_landlock*/ false,
|
||||
)
|
||||
@@ -85,6 +89,7 @@ async fn spawn_command_under_sandbox(
|
||||
command_cwd: AbsolutePathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
sandbox_cwd: &AbsolutePathBuf,
|
||||
additional_workspace_roots: &[AbsolutePathBuf],
|
||||
stdio_policy: StdioPolicy,
|
||||
env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child> {
|
||||
@@ -93,7 +98,10 @@ async fn spawn_command_under_sandbox(
|
||||
|
||||
let codex_linux_sandbox_exe = core_test_support::find_codex_linux_sandbox_exe()
|
||||
.map_err(|err| io::Error::new(io::ErrorKind::NotFound, err))?;
|
||||
let permission_profile = PermissionProfile::from_legacy_sandbox_policy(sandbox_policy);
|
||||
let mut workspace_roots = vec![sandbox_cwd.clone()];
|
||||
workspace_roots.extend_from_slice(additional_workspace_roots);
|
||||
let permission_profile = PermissionProfile::from_legacy_sandbox_policy(sandbox_policy)
|
||||
.materialize_project_roots_with_workspace_roots(&workspace_roots);
|
||||
spawn_command_under_linux_sandbox(
|
||||
codex_linux_sandbox_exe,
|
||||
command,
|
||||
@@ -145,6 +153,7 @@ async fn can_apply_linux_sandbox_policy(
|
||||
command_cwd.clone(),
|
||||
policy,
|
||||
sandbox_cwd,
|
||||
&[],
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
env,
|
||||
)
|
||||
@@ -181,7 +190,6 @@ async fn python_multiprocessing_lock_works_under_sandbox() {
|
||||
let writable_roots: Vec<AbsolutePathBuf> = vec!["/dev/shm".try_into().unwrap()];
|
||||
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots,
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
@@ -212,6 +220,7 @@ if __name__ == '__main__':
|
||||
command_cwd,
|
||||
&policy,
|
||||
&sandbox_cwd,
|
||||
&writable_roots,
|
||||
StdioPolicy::Inherit,
|
||||
sandbox_env,
|
||||
)
|
||||
@@ -255,6 +264,7 @@ async fn python_getpwuid_works_under_sandbox() {
|
||||
command_cwd,
|
||||
&policy,
|
||||
&sandbox_cwd,
|
||||
&[],
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
sandbox_env,
|
||||
)
|
||||
@@ -295,7 +305,6 @@ async fn sandbox_distinguishes_command_and_policy_cwds() {
|
||||
// writable only because it is under the sandbox policy cwd, not because it
|
||||
// is under a writable root.
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
@@ -311,6 +320,7 @@ async fn sandbox_distinguishes_command_and_policy_cwds() {
|
||||
command_root.clone(),
|
||||
&policy,
|
||||
&canonical_sandbox_root,
|
||||
&[],
|
||||
StdioPolicy::Inherit,
|
||||
sandbox_env.clone(),
|
||||
)
|
||||
@@ -342,6 +352,7 @@ async fn sandbox_distinguishes_command_and_policy_cwds() {
|
||||
command_root,
|
||||
&policy,
|
||||
&canonical_sandbox_root,
|
||||
&[],
|
||||
StdioPolicy::Inherit,
|
||||
sandbox_env,
|
||||
)
|
||||
@@ -376,7 +387,6 @@ async fn sandbox_blocks_first_time_dot_codex_creation() {
|
||||
let dot_codex = repo_root.join(".codex");
|
||||
let config_toml = dot_codex.join("config.toml");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
@@ -392,6 +402,7 @@ async fn sandbox_blocks_first_time_dot_codex_creation() {
|
||||
repo_root.clone(),
|
||||
&policy,
|
||||
&repo_root,
|
||||
&[],
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
sandbox_env,
|
||||
)
|
||||
@@ -546,6 +557,7 @@ where
|
||||
command_cwd,
|
||||
policy,
|
||||
&sandbox_cwd,
|
||||
&[],
|
||||
stdio_policy,
|
||||
HashMap::from([("IN_SANDBOX".into(), "1".into())]),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user