agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Move memories root setup out of core config (#24758)

Browse Source 
## Why

Config loading should not create or write-authorize the memories root
just because memory support exists. Memory startup is the code path that
actually materializes that tree.

## What

- Stop creating the memories root during Config load and remove it from
legacy workspace-write projections.
- Grant the memories root read access only when the memories feature and
use_memories are enabled.
- Create the memories root inside memories startup before seeding
extension instructions.
- Update config and startup tests around the ownership boundary.

## Tests

- just fmt
- just fix -p codex-core
- just fix -p codex-memories-write
- just test -p codex-core
memory_tool_makes_memories_root_readable_without_creating_or_widening_writes
workspace_write_includes_configured_writable_root_once_without_memories_root
permission_profile_override_keeps_memories_root_out_of_legacy_projection
permissions_profiles_allow_direct_write_roots_outside_workspace_root
default_permissions_profile_populates_runtime_sandbox_policy
- just test -p codex-memories-write memories_startup_creates_memory_root

Note: a broader just test -p codex-core run is not clean in this
sandbox; it hit missing test_stdio_server plus seatbelt, realtime, and
environment-sensitive failures. The changed config tests above pass.
This commit is contained in:
jif-oai
2026-05-28 11:51:24 +02:00
committed by GitHub
Unverified
parent 46946bb91c
commit d5ec93f379
5 changed files with 114 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/tests/suite/v2/remote_thread_store.rs
-1
View File
@@ -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"
codex-rs/core/src/config/config_tests.rs
+70 -21
View File
@@ -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()?;
codex-rs/core/src/config/mod.rs
+10 -77
View File
@@ -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,
codex-rs/memories/write/src/start.rs
+4
View File
@@ -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}");
}
codex-rs/memories/write/src/startup_tests.rs
+30
View File
@@ -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<ResponsesRequest> {
let deadline = Instant::now() + Duration::from_secs(10);
loop {