diff --git a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs index 9787868f5..c4e8fb924 100644 --- a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs +++ b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs @@ -228,7 +228,6 @@ fn assert_no_local_persistence_artifacts(codex_home: &Path) -> Result<()> { BTreeSet::from([ "config.toml".to_string(), "installation_id".to_string(), - "memories".to_string(), "skills".to_string(), ]), "non-local thread persistence should not create unexpected files in codex_home" diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index e8866aab3..950103562 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1554,7 +1554,6 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: .await?; let cwd_root = cwd.path().abs(); - let memories_root = codex_home.path().join("memories").abs(); assert_eq!( config.permissions.file_system_sandbox_policy(), FileSystemSandboxPolicy::restricted(vec![ @@ -1576,18 +1575,12 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: }, access: FileSystemAccessMode::Read, }, - FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: memories_root.clone(), - }, - access: FileSystemAccessMode::Write, - }, ]), ); assert_eq!( &config.legacy_sandbox_policy(), &SandboxPolicy::WorkspaceWrite { - writable_roots: vec![memories_root], + writable_roots: vec![], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -1816,7 +1809,7 @@ async fn managed_unrestricted_permission_profile_still_enables_network_requireme } #[tokio::test] -async fn permission_profile_override_applies_runtime_roots_to_legacy_projection() +async fn permission_profile_override_keeps_memories_root_out_of_legacy_projection() -> std::io::Result<()> { let codex_home = TempDir::new()?; let cwd = TempDir::new()?; @@ -1851,7 +1844,7 @@ async fn permission_profile_override_applies_runtime_roots_to_legacy_projection( let memories_root = codex_home.path().join("memories").abs(); assert!( - config + !config .permissions .file_system_sandbox_policy() .can_write_path_with_cwd(memories_root.as_path(), cwd.path()) @@ -1859,7 +1852,7 @@ async fn permission_profile_override_applies_runtime_roots_to_legacy_projection( assert_eq!( &config.legacy_sandbox_policy(), &SandboxPolicy::WorkspaceWrite { - writable_roots: vec![memories_root], + writable_roots: vec![], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -2757,9 +2750,6 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root() description: Some("Workspace access.".to_string()), }] ); - let memories_root = AbsolutePathBuf::from_absolute_path(std::fs::canonicalize( - codex_home.path().join("memories"), - )?)?; assert!( config .permissions @@ -2769,7 +2759,7 @@ async fn permissions_profiles_allow_direct_write_roots_outside_workspace_root() assert_eq!( &config.legacy_sandbox_policy(), &SandboxPolicy::WorkspaceWrite { - writable_roots: vec![external_write_path, memories_root], + writable_roots: vec![external_write_path], network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -4514,13 +4504,15 @@ async fn sqlite_home_defaults_to_codex_home_for_workspace_write() -> std::io::Re } #[tokio::test] -async fn workspace_write_always_includes_memories_root_once() -> std::io::Result<()> { +async fn workspace_write_includes_configured_writable_root_once_without_memories_root() +-> std::io::Result<()> { let codex_home = TempDir::new()?; let memories_root = codex_home.path().join("memories"); + let writable_root = codex_home.path().join("writable").abs(); let config = Config::load_from_base_config_with_overrides( ConfigToml { sandbox_workspace_write: Some(SandboxWorkspaceWrite { - writable_roots: vec![memories_root.abs()], + writable_roots: vec![writable_root.clone(), writable_root.clone()], ..Default::default() }), ..Default::default() @@ -4540,21 +4532,22 @@ async fn workspace_write_always_includes_memories_root_once() -> std::io::Result } } else { assert!( - memories_root.is_dir(), - "expected memories root directory to exist at {}", + !memories_root.exists(), + "expected config load not to create memories root at {}", memories_root.display() ); let expected_memories_root = memories_root.abs(); match &config.legacy_sandbox_policy() { SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { + assert!(!writable_roots.contains(&expected_memories_root)); assert_eq!( writable_roots .iter() - .filter(|root| **root == expected_memories_root) + .filter(|root| **root == writable_root) .count(), 1, "expected single writable root entry for {}", - expected_memories_root.display() + writable_root.display() ); } other => panic!("expected workspace-write policy, got {other:?}"), @@ -4564,6 +4557,62 @@ async fn workspace_write_always_includes_memories_root_once() -> std::io::Result Ok(()) } +#[tokio::test] +async fn memory_tool_makes_memories_root_readable_without_creating_or_widening_writes() +-> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + let memories_root = codex_home.path().join("memories"); + let memories_root_abs = memories_root.abs(); + + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + features: Some(FeaturesToml::from(BTreeMap::from([( + "memories".to_string(), + true, + )]))), + sandbox_workspace_write: Some(SandboxWorkspaceWrite { + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + ..Default::default() + }), + ..Default::default() + }, + ConfigOverrides { + cwd: Some(cwd.path().to_path_buf()), + sandbox_mode: Some(SandboxMode::WorkspaceWrite), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert!( + !memories_root.exists(), + "expected config load not to create memories root at {}", + memories_root.display() + ); + let file_system_policy = config.permissions.file_system_sandbox_policy(); + assert!(file_system_policy.can_read_path_with_cwd(memories_root_abs.as_path(), cwd.path())); + assert!(!file_system_policy.can_write_path_with_cwd(memories_root_abs.as_path(), cwd.path())); + + if cfg!(target_os = "windows") { + match &config.legacy_sandbox_policy() { + SandboxPolicy::ReadOnly { .. } => {} + other => panic!("expected read-only policy on Windows, got {other:?}"), + } + } else { + match &config.legacy_sandbox_policy() { + SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { + assert!(!writable_roots.contains(&memories_root_abs)); + } + other => panic!("expected workspace-write policy, got {other:?}"), + } + } + + Ok(()) +} + #[tokio::test] async fn config_defaults_to_file_cli_auth_store_mode() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 3115921c2..618520b7b 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2631,9 +2631,8 @@ impl Config { Some(WindowsSandboxModeToml::Unelevated) => WindowsSandboxLevel::RestrictedToken, None => WindowsSandboxLevel::from_features(&features), }; + let memories_config: MemoriesConfig = cfg.memories.clone().unwrap_or_default().into(); let memories_root = memory_root(&codex_home); - std::fs::create_dir_all(&memories_root)?; - let internal_writable_roots = vec![memories_root]; let profiles_are_active = effective_permission_selection.profiles_are_active( default_permissions_override.as_deref(), @@ -2701,8 +2700,8 @@ impl Config { file_system_sandbox_policy, mut active_permission_profile, mut profile_workspace_roots, - ) = if let Some(mut permission_profile) = permission_profile { - let (mut file_system_sandbox_policy, network_sandbox_policy) = + ) = if let Some(permission_profile) = permission_profile { + let (file_system_sandbox_policy, _network_sandbox_policy) = permission_profile.to_runtime_permissions(); let configured_network_proxy_config = if profile_allows_configured_network_proxy(&permission_profile) @@ -2726,30 +2725,6 @@ impl Config { } else { NetworkProxyConfig::default() }; - let materialized_file_system_sandbox_policy = file_system_sandbox_policy - .clone() - .materialize_project_roots_with_workspace_roots(&workspace_roots); - let materialized_permission_profile = - PermissionProfile::from_runtime_permissions_with_enforcement( - permission_profile.enforcement(), - &materialized_file_system_sandbox_policy, - network_sandbox_policy, - ); - let sandbox_policy = compatibility_sandbox_policy_for_permission_profile( - &materialized_permission_profile, - &materialized_file_system_sandbox_policy, - network_sandbox_policy, - resolved_cwd.as_path(), - ); - if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { - file_system_sandbox_policy = file_system_sandbox_policy - .with_additional_legacy_workspace_writable_roots(&internal_writable_roots); - permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( - permission_profile.enforcement(), - &file_system_sandbox_policy, - network_sandbox_policy, - ); - } ( configured_network_proxy_config, permission_profile, @@ -2794,7 +2769,7 @@ impl Config { dedupe_absolute_paths(&mut configured_workspace_roots); file_system_sandbox_policy = file_system_sandbox_policy .with_materialized_project_roots_for_workspace_roots(&configured_workspace_roots); - let mut permission_profile = if let Some(permission_profile) = + let permission_profile = if let Some(permission_profile) = builtin_permission_profile(default_permissions, builtin_workspace_write_settings) { permission_profile @@ -2804,30 +2779,6 @@ impl Config { network_sandbox_policy, ) }; - let materialized_file_system_sandbox_policy = file_system_sandbox_policy - .clone() - .materialize_project_roots_with_workspace_roots(&workspace_roots); - let materialized_permission_profile = - PermissionProfile::from_runtime_permissions_with_enforcement( - permission_profile.enforcement(), - &materialized_file_system_sandbox_policy, - network_sandbox_policy, - ); - let sandbox_policy = compatibility_sandbox_policy_for_permission_profile( - &materialized_permission_profile, - &materialized_file_system_sandbox_policy, - network_sandbox_policy, - resolved_cwd.as_path(), - ); - if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) { - file_system_sandbox_policy = file_system_sandbox_policy - .with_additional_legacy_workspace_writable_roots(&internal_writable_roots); - permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( - permission_profile.enforcement(), - &file_system_sandbox_policy, - network_sandbox_policy, - ); - } let active_permission_profile = if using_implicit_builtin_profile && default_permissions == BUILT_IN_WORKSPACE_PROFILE && cfg.sandbox_workspace_write.is_some() @@ -2885,29 +2836,8 @@ impl Config { ); permission_profile = PermissionProfile::read_only(); } - let (mut file_system_sandbox_policy, network_sandbox_policy) = + let (file_system_sandbox_policy, _network_sandbox_policy) = permission_profile.to_runtime_permissions(); - let materialized_file_system_sandbox_policy = permission_profile - .clone() - .materialize_project_roots_with_workspace_roots(&workspace_roots) - .file_system_sandbox_policy(); - if matches!(permission_profile.enforcement(), SandboxEnforcement::Managed) - && materialized_file_system_sandbox_policy.can_write_path_with_cwd( - resolved_cwd.as_path(), - resolved_cwd.as_path(), - ) - && !materialized_file_system_sandbox_policy.has_full_disk_write_access() - { - // Keep Codex runtime write access while storing the runtime - // workspace roots separately on the thread. - file_system_sandbox_policy = file_system_sandbox_policy - .with_additional_legacy_workspace_writable_roots(&internal_writable_roots); - permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( - permission_profile.enforcement(), - &file_system_sandbox_policy, - network_sandbox_policy, - ); - } ( configured_network_proxy_config, permission_profile, @@ -3324,11 +3254,14 @@ impl Config { network_requirements, &network_permission_profile, )?; - let helper_readable_roots = get_readable_roots_required_for_codex_runtime( + let mut helper_readable_roots = get_readable_roots_required_for_codex_runtime( &codex_home, zsh_path.as_ref(), main_execve_wrapper_exe.as_ref(), ); + if features.enabled(Feature::MemoryTool) && memories_config.use_memories { + helper_readable_roots.push(memories_root); + } let effective_permission_profile = constrained_permission_profile.value.get().clone(); let (mut effective_file_system_sandbox_policy, effective_network_sandbox_policy) = effective_permission_profile.to_runtime_permissions(); @@ -3438,7 +3371,7 @@ impl Config { agent_max_threads, agent_max_depth, agent_roles, - memories: cfg.memories.unwrap_or_default().into(), + memories: memories_config, agent_job_max_runtime_seconds, agent_interrupt_message_enabled, codex_home, diff --git a/codex-rs/memories/write/src/start.rs b/codex-rs/memories/write/src/start.rs index 007f5f8bb..809bf775b 100644 --- a/codex-rs/memories/write/src/start.rs +++ b/codex-rs/memories/write/src/start.rs @@ -50,6 +50,10 @@ pub fn start_memories_startup_task( tokio::spawn(async move { let root = memory_root(&config.codex_home); + if let Err(err) = tokio::fs::create_dir_all(&root).await { + warn!("failed creating memories root: {err}"); + return; + } if let Err(err) = seed_extension_instructions(&root).await { warn!("failed seeding memory extension instructions: {err}"); } diff --git a/codex-rs/memories/write/src/startup_tests.rs b/codex-rs/memories/write/src/startup_tests.rs index 1a735b366..bd0a9904e 100644 --- a/codex-rs/memories/write/src/startup_tests.rs +++ b/codex-rs/memories/write/src/startup_tests.rs @@ -26,6 +26,21 @@ use tempfile::TempDir; use tokio::time::Duration; use tokio::time::Instant; +#[tokio::test] +async fn memories_startup_creates_memory_root() -> anyhow::Result<()> { + let server = start_mock_server().await; + let home = Arc::new(TempDir::new()?); + let memory_root = home.path().join("memories"); + let test = build_test_codex(&server, home).await?; + + assert!(!memory_root.exists()); + trigger_memories_startup(&test).await; + wait_for_dir(&memory_root).await?; + + shutdown_test_codex(&test).await?; + Ok(()) +} + #[tokio::test] async fn memories_startup_phase2_tracks_workspace_diff_across_runs() -> anyhow::Result<()> { let server = start_mock_server().await; @@ -408,6 +423,21 @@ async fn wait_for_file_removed(path: &Path) -> anyhow::Result<()> { } } +async fn wait_for_dir(path: &Path) -> anyhow::Result<()> { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + if tokio::fs::try_exists(path).await? && path.is_dir() { + return Ok(()); + } + assert!( + Instant::now() < deadline, + "timed out waiting for {} to be created", + path.display() + ); + tokio::time::sleep(Duration::from_millis(50)).await; + } +} + async fn wait_for_request(mock: &ResponseMock, expected_count: usize) -> Vec { let deadline = Instant::now() + Duration::from_secs(10); loop {