diff --git a/codex-rs/memories/write/src/lib.rs b/codex-rs/memories/write/src/lib.rs index ff86068ba6..c4e4da6463 100644 --- a/codex-rs/memories/write/src/lib.rs +++ b/codex-rs/memories/write/src/lib.rs @@ -130,5 +130,18 @@ pub fn raw_memories_file(root: &Path) -> PathBuf { } pub async fn ensure_layout(root: &Path) -> std::io::Result<()> { + tokio::fs::create_dir_all(root).await?; + if tokio::fs::symlink_metadata(root) + .await? + .file_type() + .is_symlink() + { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("memory root cannot be a symbolic link: {}", root.display()), + )); + } + + workspace::remove_memory_symlinks(root).await?; tokio::fs::create_dir_all(rollout_summaries_dir(root)).await } diff --git a/codex-rs/memories/write/src/phase2.rs b/codex-rs/memories/write/src/phase2.rs index ac5cdd5ed5..b3c0fbb245 100644 --- a/codex-rs/memories/write/src/phase2.rs +++ b/codex-rs/memories/write/src/phase2.rs @@ -11,6 +11,7 @@ use crate::runtime::SpawnedConsolidationAgent; use crate::sync_rollout_summaries_from_memories; use crate::workspace::memory_workspace_diff; use crate::workspace::prepare_memory_workspace; +use crate::workspace::remove_memory_symlinks; use crate::workspace::reset_memory_workspace_baseline; use crate::workspace::validate_consolidation_artifacts; use crate::workspace::write_workspace_diff; @@ -478,6 +479,9 @@ mod agent { } } } else if !agent_completed { + if let Err(err) = remove_memory_symlinks(&memory_root).await { + tracing::error!("failed removing memory workspace symbolic links: {err}"); + } job::failed(context.as_ref(), &db, &claim, "failed_agent").await; } }); diff --git a/codex-rs/memories/write/src/start.rs b/codex-rs/memories/write/src/start.rs index d4bad64da7..d7c964bd23 100644 --- a/codex-rs/memories/write/src/start.rs +++ b/codex-rs/memories/write/src/start.rs @@ -1,3 +1,4 @@ +use crate::ensure_layout; use crate::extensions::seed_extension_instructions; use crate::guard; use crate::memory_root; @@ -52,8 +53,8 @@ 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}"); + if let Err(err) = ensure_layout(&root).await { + warn!("failed preparing memories root: {err}"); return; } if let Err(err) = seed_extension_instructions(&root).await { diff --git a/codex-rs/memories/write/src/startup_tests.rs b/codex-rs/memories/write/src/startup_tests.rs index df18be0f27..37dcf1a2c5 100644 --- a/codex-rs/memories/write/src/startup_tests.rs +++ b/codex-rs/memories/write/src/startup_tests.rs @@ -37,7 +37,11 @@ use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; +#[cfg(unix)] +use core_test_support::responses::mount_sse_once_match; use core_test_support::responses::sse; +#[cfg(unix)] +use core_test_support::responses::sse_failed; use core_test_support::responses::start_mock_server; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::test_codex; @@ -65,6 +69,110 @@ async fn memories_startup_creates_memory_root() -> anyhow::Result<()> { Ok(()) } +#[cfg(unix)] +#[tokio::test] +async fn memories_startup_removes_symlinked_extensions_before_seeding() -> anyhow::Result<()> { + let server = start_mock_server().await; + let home = Arc::new(TempDir::new()?); + let memory_root = home.path().join("memories"); + let outside = home.path().join("outside"); + tokio::fs::create_dir_all(&memory_root).await?; + tokio::fs::create_dir_all(&outside).await?; + std::os::unix::fs::symlink(&outside, memory_root.join("extensions"))?; + + let test = build_test_codex(&server, home).await?; + trigger_memories_startup(&test).await; + wait_for_dir(&memory_root.join("extensions/ad_hoc")).await?; + + assert!( + tokio::fs::symlink_metadata(memory_root.join("extensions")) + .await? + .is_dir() + ); + assert!( + !outside.join("ad_hoc").exists(), + "extension seeding must not create directories through the removed symbolic link" + ); + + shutdown_test_codex(&test).await?; + Ok(()) +} + +#[cfg(unix)] +#[tokio::test] +async fn memories_startup_fails_consolidation_when_worker_creates_extension_symlink() +-> anyhow::Result<()> { + let responses = [ + sse(vec![ + ev_response_created("resp-phase2-complete"), + ev_assistant_message("msg-phase2-complete", "phase2 complete"), + ev_completed("resp-phase2-complete"), + ]), + sse_failed("resp-phase2-failed", "server_error", "worker failed"), + ]; + + for response in responses { + let server = start_mock_server().await; + let home = Arc::new(TempDir::new()?); + let db = init_state_db(&home).await?; + let root = home.path().join("memories"); + seed_stage1_output( + db.as_ref(), + home.path(), + chrono::Utc::now(), + "raw memory", + "rollout summary", + "worker-symlink", + ) + .await?; + seed_required_memory_artifacts(&root).await?; + reset_git_repository(&root).await?; + + let target = home.path().join("outside.md"); + tokio::fs::write(&target, "outside content").await?; + let link = root.join("extensions/external_agent_import/instructions.md"); + let worker_target = target.clone(); + let worker_link = link.clone(); + let phase2 = mount_sse_once_match( + &server, + move |_request: &wiremock::Request| { + std::fs::create_dir_all(worker_link.parent().expect("extension directory")) + .expect("create extension directory"); + std::os::unix::fs::symlink(&worker_target, &worker_link) + .expect("create worker symbolic link"); + true + }, + response, + ) + .await; + let test = build_test_codex(&server, home).await?; + + trigger_memories_startup(&test).await; + wait_for_single_request(&phase2).await; + + assert_eq!( + wait_for_phase2_job_to_finish(db.as_ref()).await?, + Phase2JobClaimOutcome::SkippedRetryUnavailable + ); + assert_eq!(tokio::fs::read_to_string(&target).await?, "outside content"); + assert_eq!( + tokio::fs::symlink_metadata(&link) + .await + .expect_err("worker-created symbolic link should be removed") + .kind(), + std::io::ErrorKind::NotFound + ); + assert!( + root.join("phase2_workspace_diff.md").exists(), + "a rejected consolidation result must not reset the trusted baseline" + ); + + 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; diff --git a/codex-rs/memories/write/src/workspace.rs b/codex-rs/memories/write/src/workspace.rs index 9f80693e47..1b51b05290 100644 --- a/codex-rs/memories/write/src/workspace.rs +++ b/codex-rs/memories/write/src/workspace.rs @@ -3,6 +3,8 @@ use codex_git_utils::GitBaselineDiff; use codex_git_utils::diff_since_latest_init; use codex_git_utils::ensure_git_baseline_repository; use codex_git_utils::reset_git_repository; +#[cfg(windows)] +use std::os::windows::fs::FileTypeExt; use std::path::Path; /// Prepares the memory directory for git-baseline diffing. @@ -11,9 +13,9 @@ use std::path::Path; /// metadata is missing or unusable, and removes any stale generated `phase2_workspace_diff.md` file /// so that the next diff does not include a previous prompt artifact. pub async fn prepare_memory_workspace(root: &Path) -> anyhow::Result<()> { - tokio::fs::create_dir_all(root) + crate::ensure_layout(root) .await - .with_context(|| format!("create memory workspace {}", root.display()))?; + .with_context(|| format!("prepare memory workspace {}", root.display()))?; remove_workspace_diff(root).await?; ensure_git_baseline_repository(root).await?; Ok(()) @@ -47,6 +49,12 @@ pub async fn reset_memory_workspace_baseline(root: &Path) -> anyhow::Result<()> /// Verifies that a completed consolidation run left the required memory artifacts in place. pub async fn validate_consolidation_artifacts(root: &Path) -> anyhow::Result<()> { + let removed_symlinks = remove_memory_symlinks(root).await?; + anyhow::ensure!( + removed_symlinks == 0, + "removed {removed_symlinks} symbolic links from consolidated memory workspace" + ); + let memory_path = root.join("MEMORY.md"); let memory_metadata = tokio::fs::metadata(&memory_path).await.with_context(|| { format!( @@ -73,6 +81,39 @@ pub async fn validate_consolidation_artifacts(root: &Path) -> anyhow::Result<()> Ok(()) } +pub(crate) async fn remove_memory_symlinks(root: &Path) -> std::io::Result { + let mut directories = vec![root.to_path_buf()]; + let mut removed = 0; + + while let Some(directory) = directories.pop() { + let mut entries = tokio::fs::read_dir(directory).await?; + while let Some(entry) = entries.next_entry().await? { + let path = entry.path(); + let file_type = entry.file_type().await?; + if file_type.is_symlink() { + #[cfg(windows)] + if file_type.is_symlink_dir() { + tokio::fs::remove_dir(&path).await?; + } else { + tokio::fs::remove_file(&path).await?; + } + #[cfg(not(windows))] + tokio::fs::remove_file(&path).await?; + + tracing::warn!( + "removed symbolic link from memory workspace: {}", + path.display() + ); + removed += 1; + } else if file_type.is_dir() { + directories.push(path); + } + } + } + + Ok(removed) +} + /// Removes the generated `phase2_workspace_diff.md` prompt artifact. /// /// This does not remove `.git/`, reset the baseline, or delete memory content. It is used before