From f72ab43fd193b31208cd3c306293b1b71a52a709 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 4 Mar 2026 13:00:26 +0000 Subject: [PATCH] feat: memories in workspace write (#13467) --- codex-rs/README.md | 1 + codex-rs/core/src/codex.rs | 6 +-- codex-rs/core/src/config/mod.rs | 60 ++++++++++++++++++++++++ codex-rs/core/src/memories/control.rs | 33 +++++++++++++ codex-rs/core/src/memories/mod.rs | 2 + codex-rs/core/src/memories/tests.rs | 67 +++++++++++++++++++++++++++ 6 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 codex-rs/core/src/memories/control.rs diff --git a/codex-rs/README.md b/codex-rs/README.md index 1c93852fe..ad8a0506f 100644 --- a/codex-rs/README.md +++ b/codex-rs/README.md @@ -88,6 +88,7 @@ codex --sandbox danger-full-access ``` The same setting can be persisted in `~/.codex/config.toml` via the top-level `sandbox_mode = "MODE"` key, e.g. `sandbox_mode = "workspace-write"`. +In `workspace-write`, Codex also includes `~/.codex/memories` in its writable roots so memory maintenance does not require an extra approval. ## Code Organization diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0ac2b14f4..6cb7b0f61 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -4454,11 +4454,9 @@ mod handlers { } let memory_root = crate::memories::memory_root(&config.codex_home); - if let Err(err) = tokio::fs::remove_dir_all(&memory_root).await - && err.kind() != std::io::ErrorKind::NotFound - { + if let Err(err) = crate::memories::clear_memory_root_contents(&memory_root).await { errors.push(format!( - "failed removing memory directory {}: {err}", + "failed clearing memory directory {}: {err}", memory_root.display() )); } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index ca99cce54..7fe54289e 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -40,6 +40,7 @@ use crate::features::FeatureOverrides; use crate::features::Features; use crate::features::FeaturesToml; use crate::git_info::resolve_root_git_project_for_trust; +use crate::memories::memory_root; use crate::model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID; use crate::model_provider_info::LMSTUDIO_OSS_PROVIDER_ID; use crate::model_provider_info::ModelProviderInfo; @@ -1805,6 +1806,15 @@ impl Config { Some(&constrained_sandbox_policy), ); if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy { + let memories_root = memory_root(&codex_home); + std::fs::create_dir_all(&memories_root)?; + let memories_root = AbsolutePathBuf::from_absolute_path(&memories_root)?; + if !writable_roots + .iter() + .any(|existing| existing == &memories_root) + { + writable_roots.push(memories_root); + } for path in additional_writable_roots { if !writable_roots.iter().any(|existing| existing == &path) { writable_roots.push(path); @@ -3156,6 +3166,56 @@ trust_level = "trusted" Ok(()) } + #[test] + fn workspace_write_always_includes_memories_root_once() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let memories_root = codex_home.path().join("memories"); + let config = Config::load_from_base_config_with_overrides( + ConfigToml { + sandbox_workspace_write: Some(SandboxWorkspaceWrite { + writable_roots: vec![AbsolutePathBuf::from_absolute_path(&memories_root)?], + ..Default::default() + }), + ..Default::default() + }, + ConfigOverrides { + sandbox_mode: Some(SandboxMode::WorkspaceWrite), + ..Default::default() + }, + codex_home.path().to_path_buf(), + )?; + + if cfg!(target_os = "windows") { + match config.permissions.sandbox_policy.get() { + SandboxPolicy::ReadOnly { .. } => {} + other => panic!("expected read-only policy on Windows, got {other:?}"), + } + } else { + assert!( + memories_root.is_dir(), + "expected memories root directory to exist at {}", + memories_root.display() + ); + let expected_memories_root = AbsolutePathBuf::from_absolute_path(&memories_root)?; + match config.permissions.sandbox_policy.get() { + SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { + assert_eq!( + writable_roots + .iter() + .filter(|root| **root == expected_memories_root) + .count(), + 1, + "expected single writable root entry for {}", + expected_memories_root.display() + ); + } + other => panic!("expected workspace-write policy, got {other:?}"), + } + } + + Ok(()) + } + #[test] fn config_defaults_to_file_cli_auth_store_mode() -> std::io::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/memories/control.rs b/codex-rs/core/src/memories/control.rs new file mode 100644 index 000000000..10bd19246 --- /dev/null +++ b/codex-rs/core/src/memories/control.rs @@ -0,0 +1,33 @@ +use std::path::Path; + +pub(crate) async fn clear_memory_root_contents(memory_root: &Path) -> std::io::Result<()> { + match tokio::fs::symlink_metadata(memory_root).await { + Ok(metadata) if metadata.file_type().is_symlink() => { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "refusing to clear symlinked memory root {}", + memory_root.display() + ), + )); + } + Ok(_) => {} + Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} + Err(err) => return Err(err), + } + + tokio::fs::create_dir_all(memory_root).await?; + + let mut entries = tokio::fs::read_dir(memory_root).await?; + while let Some(entry) = entries.next_entry().await? { + let path = entry.path(); + let file_type = entry.file_type().await?; + if file_type.is_dir() { + tokio::fs::remove_dir_all(path).await?; + } else { + tokio::fs::remove_file(path).await?; + } + } + + Ok(()) +} diff --git a/codex-rs/core/src/memories/mod.rs b/codex-rs/core/src/memories/mod.rs index a90eba6be..83604b6af 100644 --- a/codex-rs/core/src/memories/mod.rs +++ b/codex-rs/core/src/memories/mod.rs @@ -5,6 +5,7 @@ //! - Phase 2: claim a global consolidation lock, materialize consolidation inputs, and dispatch one consolidation agent. pub(crate) mod citations; +mod control; mod phase1; mod phase2; pub(crate) mod prompts; @@ -16,6 +17,7 @@ pub(crate) mod usage; use codex_protocol::openai_models::ReasoningEffort; +pub(crate) use control::clear_memory_root_contents; /// Starts the memory startup pipeline for eligible root sessions. /// This is the single entrypoint that `codex` uses to trigger memory startup. /// diff --git a/codex-rs/core/src/memories/tests.rs b/codex-rs/core/src/memories/tests.rs index 0ebc0b8f1..1c0fa7b29 100644 --- a/codex-rs/core/src/memories/tests.rs +++ b/codex-rs/core/src/memories/tests.rs @@ -1,6 +1,7 @@ use super::storage::rebuild_raw_memories_file_from_memories; use super::storage::sync_rollout_summaries_from_memories; use crate::config::types::DEFAULT_MEMORIES_MAX_RAW_MEMORIES_FOR_CONSOLIDATION; +use crate::memories::clear_memory_root_contents; use crate::memories::ensure_layout; use crate::memories::memory_root; use crate::memories::raw_memories_file; @@ -63,6 +64,72 @@ fn stage_one_output_schema_requires_rollout_slug_and_keeps_it_nullable() { assert_eq!(rollout_slug_types, vec!["null", "string"]); } +#[tokio::test] +async fn clear_memory_root_contents_preserves_root_directory() { + let dir = tempdir().expect("tempdir"); + let root = dir.path().join("memory"); + let nested_dir = root.join("rollout_summaries"); + tokio::fs::create_dir_all(&nested_dir) + .await + .expect("create rollout summaries dir"); + tokio::fs::write(root.join("MEMORY.md"), "stale memory index\n") + .await + .expect("write memory index"); + tokio::fs::write(nested_dir.join("rollout.md"), "stale rollout\n") + .await + .expect("write rollout summary"); + + clear_memory_root_contents(&root) + .await + .expect("clear memory root contents"); + + assert!( + tokio::fs::try_exists(&root) + .await + .expect("check memory root existence"), + "memory root should still exist after clearing contents" + ); + let mut entries = tokio::fs::read_dir(&root) + .await + .expect("read memory root after clear"); + assert!( + entries + .next_entry() + .await + .expect("read next entry") + .is_none(), + "memory root should be empty after clearing contents" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn clear_memory_root_contents_rejects_symlinked_root() { + let dir = tempdir().expect("tempdir"); + let target = dir.path().join("outside"); + tokio::fs::create_dir_all(&target) + .await + .expect("create symlink target dir"); + let target_file = target.join("keep.txt"); + tokio::fs::write(&target_file, "keep\n") + .await + .expect("write target file"); + + let root = dir.path().join("memory"); + std::os::unix::fs::symlink(&target, &root).expect("create memory root symlink"); + + let err = clear_memory_root_contents(&root) + .await + .expect_err("symlinked memory root should be rejected"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + tokio::fs::try_exists(&target_file) + .await + .expect("check target file existence"), + "rejecting a symlinked memory root should not delete the symlink target" + ); +} + #[tokio::test] async fn sync_rollout_summaries_and_raw_memories_file_keeps_latest_memories_only() { let dir = tempdir().expect("tempdir");