From 7a13323a41bc1e2bf43f7d4c369cab06064ebbe5 Mon Sep 17 00:00:00 2001 From: joshyorko Date: Thu, 27 Aug 2026 08:39:20 -0400 Subject: [PATCH 1/4] feat: make Codex MCP launcher runtime-aware --- src/cli.rs | 260 ++++++++++++++++++- src/native_runtime.rs | 2 +- tests/cli_smoke.rs | 580 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 829 insertions(+), 13 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 403506f..decada4 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -4,6 +4,7 @@ //! launches the daemon; `doctor` runs self-checks. use std::collections::BTreeMap; +use std::fmt::Write as _; use std::path::Path; use std::path::PathBuf; @@ -2406,7 +2407,7 @@ struct CodexMcpConfigReport { server_name: &'static str, backup_file: Option, snippet: String, - resolved: CodexMcpResolved, + resolved: Option, } #[derive(Debug, Serialize)] @@ -2427,14 +2428,28 @@ fn manage_codex_mcp_config( let config_path = codex_config_override .map(Path::to_path_buf) .unwrap_or_else(default_codex_config_path); - let desired = render_codex_mcp_snippet(runtime); - let raw = std::fs::read_to_string(&config_path).unwrap_or_default(); + let (desired, resolved) = if matches!(command, CodexMcpCommand::Remove) { + removal_payload(runtime) + } else { + let resolved = codex_mcp_resolved(runtime)?; + let desired = render_codex_mcp_snippet(&resolved)?; + (desired, Some(resolved)) + }; + let raw = match std::fs::read_to_string(&config_path) { + Ok(raw) => raw, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(err) => { + return Err(error::Error::invalid_request(format!( + "cannot read Codex config {}: {err}; check --codex-config or CODEX_HOME permissions", + config_path.display() + ))) + } + }; let config_exists = config_path.exists(); let section = find_codex_mcp_section(&raw); let matches_desired = section .as_ref() .is_some_and(|(start, end)| raw[*start..*end].trim_end() == desired.trim_end()); - let resolved = codex_mcp_resolved(runtime); let status = match (section.is_some(), matches_desired) { (false, _) => "missing", (true, true) => "managed", @@ -2531,7 +2546,20 @@ fn manage_codex_mcp_config( } } -fn codex_mcp_resolved(runtime: &RuntimeOptions) -> CodexMcpResolved { +fn removal_payload(runtime: &RuntimeOptions) -> (String, Option) { + if runtime.runtime == RuntimeKind::Container { + // Removal must remain available when the old Docker/Podman executable + // has been uninstalled. There is no useful resolved launcher to report + // without resolving that executable, so omit it from this report. + return (String::new(), None); + } + + let resolved = native_codex_mcp_resolved(runtime); + let desired = render_codex_mcp_snippet(&resolved).unwrap_or_default(); + (desired, Some(resolved)) +} + +fn native_codex_mcp_resolved(runtime: &RuntimeOptions) -> CodexMcpResolved { CodexMcpResolved { command: runtime.binary.display().to_string(), args: vec![ @@ -2548,13 +2576,223 @@ fn codex_mcp_resolved(runtime: &RuntimeOptions) -> CodexMcpResolved { } } -fn render_codex_mcp_snippet(runtime: &RuntimeOptions) -> String { - format!( - "{header}\ncommand = {command:?}\nargs = [\"--db\", {db:?}, \"mcp\", \"stdio\", \"--read-only\"]\nenabled_tools = [\"memory_status\", \"memory_recall\", \"memory_search\"]\ndefault_tools_approval_mode = \"approve\"\nstartup_timeout_sec = 30\ntool_timeout_sec = 30\n", +fn codex_mcp_resolved(runtime: &RuntimeOptions) -> Result { + match runtime.runtime { + RuntimeKind::Native | RuntimeKind::Auto => Ok(native_codex_mcp_resolved(runtime)), + RuntimeKind::ComposeDev => Err(error::Error::invalid_request( + "Codex MCP does not support --runtime compose-dev; choose --runtime native or --runtime container", + )), + RuntimeKind::Container => { + let command = native_runtime::container_runtime(runtime).map_err(|err| { + error::Error::invalid_request(format!( + "container MCP requires an available Docker or Podman runtime: {}. Install Docker or Podman, or set CODEX_MEMORYD_CONTAINER_RUNTIME to an executable path", + err.message + )) + })?; + let (database_parent, database_filename) = container_mcp_database(runtime)?; + let uid = container_mcp_id( + runtime.uid.as_deref(), + "CODEX_MEMORYD_UID", + "id -u", + )?; + let gid = container_mcp_id( + runtime.gid.as_deref(), + "CODEX_MEMORYD_GID", + "id -g", + )?; + + let args = vec![ + "run".to_string(), + "--rm".to_string(), + "-i".to_string(), + "--pull=missing".to_string(), + "--user".to_string(), + format!("{uid}:{gid}"), + "--volume".to_string(), + format!("{database_parent}:/data"), + runtime.image.clone(), + "--db".to_string(), + format!("/data/{database_filename}"), + "mcp".to_string(), + "stdio".to_string(), + "--read-only".to_string(), + ]; + + Ok(CodexMcpResolved { + command, + args, + enabled_tools: CODEX_MCP_READ_ONLY_TOOLS.to_vec(), + default_tools_approval_mode: "approve", + startup_timeout_sec: 30, + tool_timeout_sec: 30, + }) + } + } +} + +fn container_mcp_id(value: Option<&str>, variable: &str, fallback: &str) -> Result { + let Some(value) = value else { + return Err(error::Error::invalid_request(format!( + "container MCP requires a host UID/GID; set {variable}= or make {fallback} available" + ))); + }; + let value = value.trim(); + if value.is_empty() + || !value.chars().all(|character| character.is_ascii_digit()) + || value.parse::().is_err() + { + return Err(error::Error::invalid_request(format!( + "container MCP requires a valid numeric value for {variable}; set {variable}= or make {fallback} available" + ))); + } + Ok(value + .parse::() + .expect("validated numeric id") + .to_string()) +} + +fn container_mcp_database(runtime: &RuntimeOptions) -> Result<(String, String)> { + let database = if runtime.db.is_absolute() { + runtime.db.clone() + } else { + std::env::current_dir() + .map_err(|err| { + error::Error::invalid_request(format!( + "container MCP cannot resolve relative database path {}: {err}; set CODEX_MEMORYD_DB to an absolute path", + runtime.db.display() + )) + })? + .join(&runtime.db) + }; + let filename = database + .file_name() + .and_then(|name| name.to_str()) + .filter(|name| !name.is_empty() && *name != "." && *name != "..") + .ok_or_else(|| { + error::Error::invalid_request(format!( + "container MCP database path must name a UTF-8 file: {}; set CODEX_MEMORYD_DB=/path/to/memory.db", + database.display() + )) + })?; + if filename.chars().any(char::is_control) { + return Err(error::Error::invalid_request( + "container MCP database filename contains control characters and cannot be represented safely; set CODEX_MEMORYD_DB to a normal filename", + )); + } + + let parent = database.parent().ok_or_else(|| { + error::Error::invalid_request(format!( + "container MCP database path has no usable parent directory: {}; set CODEX_MEMORYD_DB=/path/to/memory.db", + database.display() + )) + })?; + let metadata = std::fs::metadata(parent).map_err(|err| { + if err.kind() == std::io::ErrorKind::NotFound { + error::Error::invalid_request(format!( + "container MCP database parent directory does not exist: {}; create it with mkdir -p {} or set CODEX_MEMORYD_DB to an existing directory", + parent.display(), + parent.display() + )) + } else { + error::Error::invalid_request(format!( + "container MCP database parent directory is not usable: {} ({err}); check permissions or set CODEX_MEMORYD_DB to an accessible directory", + parent.display() + )) + } + })?; + if !metadata.is_dir() { + return Err(error::Error::invalid_request(format!( + "container MCP database parent is not a directory: {}; set CODEX_MEMORYD_DB to a file inside an existing directory", + parent.display() + ))); + } + + let parent = std::fs::canonicalize(parent).map_err(|err| { + error::Error::invalid_request(format!( + "container MCP database parent cannot be resolved safely: {} ({err}); set CODEX_MEMORYD_DB to an accessible absolute path", + parent.display() + )) + })?; + if parent == Path::new("/") { + return Err(error::Error::invalid_request( + "container MCP refuses to mount the host root as the database parent; set CODEX_MEMORYD_DB inside a dedicated data directory", + )); + } + let parent = parent.to_str().ok_or_else(|| { + error::Error::invalid_request( + "container MCP database parent is not valid UTF-8 and cannot be represented safely; set CODEX_MEMORYD_DB to a UTF-8 path", + ) + })?; + if parent.contains(':') || parent.chars().any(char::is_control) { + return Err(error::Error::invalid_request(format!( + "container MCP database parent cannot be represented safely in a Docker/Podman --volume mount: {parent}; use a path without ':' or control characters, or set CODEX_MEMORYD_DB to a safe directory" + ))); + } + Ok((parent.to_string(), filename.to_string())) +} + +fn toml_basic_string(value: &str, label: &str) -> Result { + let mut escaped = String::with_capacity(value.len() + 2); + for character in value.chars() { + match character { + '\0' => { + return Err(error::Error::invalid_request(format!( + "generated Codex MCP TOML cannot represent {label} containing NUL safely; set the corresponding CODEX_MEMORYD_* override to ordinary text" + ))) + } + '"' => escaped.push_str("\\\""), + '\\' => escaped.push_str("\\\\"), + '\u{0008}' => escaped.push_str("\\b"), + '\t' => escaped.push_str("\\t"), + '\n' => escaped.push_str("\\n"), + '\u{000c}' => escaped.push_str("\\f"), + '\r' => escaped.push_str("\\r"), + character if character.is_control() => { + let codepoint = character as u32; + if codepoint <= 0xffff { + write!(&mut escaped, "\\u{codepoint:04X}") + .expect("writing String cannot fail"); + } else { + write!(&mut escaped, "\\U{codepoint:08X}") + .expect("writing String cannot fail"); + } + } + character => escaped.push(character), + } + } + Ok(format!("\"{escaped}\"")) +} + +fn render_codex_mcp_snippet(resolved: &CodexMcpResolved) -> Result { + let command = toml_basic_string(&resolved.command, "the MCP command")?; + let args = resolved + .args + .iter() + .map(|arg| toml_basic_string(arg, "an MCP argument")) + .collect::>>()? + .join(", "); + let enabled_tools = resolved + .enabled_tools + .iter() + .map(|tool| toml_basic_string(tool, "an enabled MCP tool")) + .collect::>>()? + .join(", "); + let approval_mode = toml_basic_string( + resolved.default_tools_approval_mode, + "the MCP approval mode", + )?; + let snippet = format!( + "{header}\ncommand = {command}\nargs = [{args}]\nenabled_tools = [{enabled_tools}]\ndefault_tools_approval_mode = {approval_mode}\nstartup_timeout_sec = {startup_timeout_sec}\ntool_timeout_sec = {tool_timeout_sec}\n", header = CODEX_MCP_SECTION_HEADER, - command = runtime.binary.display().to_string(), - db = runtime.db.display().to_string(), - ) + startup_timeout_sec = resolved.startup_timeout_sec, + tool_timeout_sec = resolved.tool_timeout_sec, + ); + toml::from_str::(&snippet).map_err(|err| { + error::Error::invalid_request(format!( + "generated Codex MCP configuration is not valid TOML: {err}; choose representable runtime, image, and database values" + )) + })?; + Ok(snippet) } fn default_codex_config_path() -> PathBuf { diff --git a/src/native_runtime.rs b/src/native_runtime.rs index 5a71a33..84e6a0d 100644 --- a/src/native_runtime.rs +++ b/src/native_runtime.rs @@ -785,7 +785,7 @@ fn running_pid(pid_file: &Path) -> Option { ok.then_some(pid) } -fn container_runtime(opts: &RuntimeOptions) -> Result { +pub fn container_runtime(opts: &RuntimeOptions) -> Result { if let Some(runtime) = &opts.container_runtime { if runtime.trim().eq_ignore_ascii_case("auto") { // Fall through to discovery below. diff --git a/tests/cli_smoke.rs b/tests/cli_smoke.rs index 8df6692..fdbd222 100644 --- a/tests/cli_smoke.rs +++ b/tests/cli_smoke.rs @@ -52,11 +52,28 @@ fn clear_runtime_env(command: &mut Command) { "CODEX_MEMORYD_BIND", "CODEX_MEMORYD_DB", "CODEX_MEMORYD_CONTAINER_RUNTIME", + "CODEX_MEMORYD_IMAGE", + "CODEX_MEMORYD_UID", + "CODEX_MEMORYD_GID", ] { command.env_remove(key); } } +#[cfg(unix)] +fn fake_container_runtime_named(root: &std::path::Path, name: &str) -> PathBuf { + let path = root.join(name); + fs::write( + &path, + "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then exit 0; fi\nexit 1\n", + ) + .unwrap(); + let mut permissions = fs::metadata(&path).unwrap().permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&path, permissions).unwrap(); + path +} + #[cfg(unix)] fn fake_container_runtime(root: &std::path::Path, running: bool) -> PathBuf { let path = root.join(if running { @@ -78,6 +95,24 @@ fn fake_container_runtime(root: &std::path::Path, running: bool) -> PathBuf { path } +#[cfg(unix)] +fn fake_container_runtime_with_log(root: &std::path::Path) -> (PathBuf, PathBuf) { + let path = root.join("fake-container-runtime-log"); + let log = root.join("container-runtime-invocations.log"); + fs::write( + &path, + format!( + "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"{}\"\nif [ \"$1\" = \"--version\" ]; then exit 0; fi\nexit 1\n", + log.display() + ), + ) + .unwrap(); + let mut permissions = fs::metadata(&path).unwrap().permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&path, permissions).unwrap(); + (path, log) +} + fn sentinel_listener() -> ( String, Arc, @@ -6399,6 +6434,34 @@ fn readme_keeps_first_run_path_documented() { } } +#[test] +fn getting_started_documents_the_full_mcp_onboarding_and_retention_path() { + let readme = include_str!("../README.md"); + let guide = include_str!("../docs/getting-started.md"); + assert!(readme.contains("docs/getting-started.md")); + for required in [ + "brew install joshyorko/tools/codex-memoryd", + "codex-memoryd mcp codex apply", + "codex-memoryd mcp codex status", + "Restart Codex", + "memory_status", + "codex-memoryd mcp codex preview --runtime container", + "codex-memoryd mcp codex apply --runtime container", + "codex-memoryd mcp codex status --runtime container", + "--runtime native", + "codex-memoryd mcp codex remove", + "brew uninstall codex-memoryd", + "does not delete the persistent memory database", + "Source-build fallback", + "release artifacts are not published yet", + ] { + assert!( + guide.contains(required), + "getting-started.md missing {required:?}" + ); + } +} + #[test] fn local_runtime_helper_documents_safe_runtime_contract() { let helper = include_str!("../scripts/codex-memoryd-local-runtime.sh"); @@ -6439,7 +6502,9 @@ fn cli_mcp_codex_preview_reports_snippet_without_writing() { let codex_home = codex_home_path(&dir); let codex_config = codex_config_path(&dir); - let output = bin() + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command .env("CODEX_MEMORYD_HOME", &memoryd_home) .env("CODEX_HOME", &codex_home) .args(["mcp", "codex", "preview"]) @@ -6467,6 +6532,436 @@ fn cli_mcp_codex_preview_reports_snippet_without_writing() { assert!(!codex_home.exists(), "preview must not create CODEX_HOME"); } +#[test] +fn cli_mcp_codex_native_output_remains_byte_for_byte_compatible() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db = dir.path().join("memory.db"); + + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_RUNTIME", "container") + .arg("--db") + .arg(&db) + .args(["--runtime", "native", "mcp", "codex", "preview"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let json: Value = serde_json::from_slice(&output).unwrap(); + let command = json["resolved"]["command"].as_str().unwrap(); + let expected = format!( + "[mcp_servers.codex_memoryd]\ncommand = {:?}\nargs = [\"--db\", {:?}, \"mcp\", \"stdio\", \"--read-only\"]\nenabled_tools = [\"memory_status\", \"memory_recall\", \"memory_search\"]\ndefault_tools_approval_mode = \"approve\"\nstartup_timeout_sec = 30\ntool_timeout_sec = 30\n", + command, + db.display().to_string(), + ); + assert_eq!(json["snippet"].as_str(), Some(expected.as_str())); + let _: toml::Value = toml::from_str(&expected).unwrap(); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_global_runtime_placements_and_engines_are_deterministic() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join("database"); + fs::create_dir_all(&db_dir).unwrap(); + let db = db_dir.join("memory.db"); + let docker = fake_container_runtime_named(dir.path(), "docker"); + let podman = fake_container_runtime_named(dir.path(), "podman"); + + let run = |runtime: &std::path::Path, before_subcommand: bool| { + let mut command = bin(); + clear_runtime_env(&mut command); + command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", runtime) + .env("CODEX_MEMORYD_IMAGE", "ghcr.io/example/codex-memoryd:test") + .env("CODEX_MEMORYD_UID", "1234") + .env("CODEX_MEMORYD_GID", "5678") + .arg("--db") + .arg(&db); + if before_subcommand { + command.args(["--runtime", "container", "mcp", "codex", "preview"]); + } else { + command.args(["mcp", "codex", "preview", "--runtime", "container"]); + } + let output = command.assert().success().get_output().stdout.clone(); + serde_json::from_slice::(&output).unwrap() + }; + + let docker_before = run(&docker, true); + let docker_after = run(&docker, false); + assert_eq!(docker_before, docker_after); + assert_eq!( + docker_before["resolved"]["command"], + docker.to_string_lossy().as_ref() + ); + assert_eq!( + docker_before["resolved"]["args"], + serde_json::json!([ + "run", + "--rm", + "-i", + "--pull=missing", + "--user", + "1234:5678", + "--volume", + format!("{}:/data", db_dir.canonicalize().unwrap().display()), + "ghcr.io/example/codex-memoryd:test", + "--db", + "/data/memory.db", + "mcp", + "stdio", + "--read-only" + ]) + ); + assert_eq!( + docker_before["resolved"]["enabled_tools"], + serde_json::json!(["memory_status", "memory_recall", "memory_search"]) + ); + assert_eq!( + docker_before["resolved"]["default_tools_approval_mode"], + "approve" + ); + assert_eq!(docker_before["resolved"]["startup_timeout_sec"], 30); + assert_eq!(docker_before["resolved"]["tool_timeout_sec"], 30); + let _: toml::Value = toml::from_str(docker_before["snippet"].as_str().unwrap()).unwrap(); + + let podman_output = run(&podman, true); + assert_eq!( + podman_output["resolved"]["command"], + podman.to_string_lossy().as_ref() + ); + assert_eq!( + podman_output["resolved"]["args"], + docker_before["resolved"]["args"] + ); + assert_ne!(podman_output["snippet"], docker_before["snippet"]); + assert!(podman_output["snippet"] + .as_str() + .unwrap() + .contains(&format!( + "command = {:?}", + podman.to_string_lossy().to_string() + ))); + let _: toml::Value = toml::from_str(podman_output["snippet"].as_str().unwrap()).unwrap(); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_toml_round_trips_spaces_quotes_backslashes_and_image_values() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join(r#"data "quoted" \slash"#); + fs::create_dir_all(&db_dir).unwrap(); + let db = db_dir.join(r#"memory "file" \name.db"#); + let runtime = fake_container_runtime_named(dir.path(), r#"runtime "quoted" \slash"#); + let image = r#"registry.example/memory "image" \tag"#; + + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_IMAGE", image) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db) + .args(["mcp", "codex", "preview", "--runtime", "container"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let json: Value = serde_json::from_slice(&output).unwrap(); + let parsed: toml::Value = toml::from_str(json["snippet"].as_str().unwrap()).unwrap(); + let server = parsed + .get("mcp_servers") + .and_then(|value| value.get("codex_memoryd")) + .and_then(toml::Value::as_table) + .unwrap(); + assert_eq!( + server.get("command").and_then(toml::Value::as_str), + runtime.to_str() + ); + let args = server + .get("args") + .and_then(toml::Value::as_array) + .unwrap() + .iter() + .map(|value| value.as_str().unwrap().to_string()) + .collect::>(); + assert_eq!( + args, + vec![ + "run".to_string(), + "--rm".to_string(), + "-i".to_string(), + "--pull=missing".to_string(), + "--user".to_string(), + "1001:1002".to_string(), + "--volume".to_string(), + format!("{}:/data", db_dir.canonicalize().unwrap().display()), + image.to_string(), + "--db".to_string(), + r#"/data/memory "file" \name.db"#.to_string(), + "mcp".to_string(), + "stdio".to_string(), + "--read-only".to_string(), + ] + ); +} + +#[test] +fn cli_mcp_codex_help_exposes_one_global_runtime_option() { + let output = bin().args(["mcp", "codex", "--help"]).output().unwrap(); + assert!(output.status.success()); + let help = String::from_utf8(output.stdout).unwrap(); + assert_eq!(help.matches("--runtime").count(), 1, "{help}"); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_missing_runtime_fails_before_config_mutation() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join("database"); + fs::create_dir_all(&db_dir).unwrap(); + let db = db_dir.join("memory.db"); + let missing_runtime = dir.path().join("missing-container-runtime"); + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &missing_runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db) + .args(["mcp", "codex", "preview", "--runtime", "container"]) + .output() + .unwrap(); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("available Docker or Podman runtime"), + "{stderr}" + ); + assert!( + stderr.contains("CODEX_MEMORYD_CONTAINER_RUNTIME"), + "{stderr}" + ); + assert!( + !codex_home.exists(), + "validation must precede config mutation" + ); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_invalid_database_parent_fails_before_config_mutation() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let runtime = fake_container_runtime_named(dir.path(), "docker"); + let missing_parent_db = dir.path().join("missing").join("memory.db"); + + let mut missing_parent = bin(); + clear_runtime_env(&mut missing_parent); + let missing_output = missing_parent + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&missing_parent_db) + .args(["mcp", "codex", "apply", "--runtime", "container"]) + .output() + .unwrap(); + assert!(!missing_output.status.success()); + let missing_stderr = String::from_utf8_lossy(&missing_output.stderr); + assert!(missing_stderr.contains("parent directory does not exist")); + assert!(missing_stderr.contains("mkdir -p")); + assert!( + !codex_home.exists(), + "invalid parent must not create CODEX_HOME" + ); + + let non_directory = dir.path().join("not-a-directory"); + fs::write(&non_directory, "not a directory").unwrap(); + let non_directory_db = non_directory.join("memory.db"); + let mut non_directory_command = bin(); + clear_runtime_env(&mut non_directory_command); + let non_directory_output = non_directory_command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&non_directory_db) + .args(["mcp", "codex", "status", "--runtime", "container"]) + .output() + .unwrap(); + assert!(!non_directory_output.status.success()); + let non_directory_stderr = String::from_utf8_lossy(&non_directory_output.stderr); + assert!(non_directory_stderr.contains("parent is not a directory")); + assert!( + !codex_home.exists(), + "invalid parent must not create CODEX_HOME" + ); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_unsafe_mount_path_fails_before_config_mutation() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let unsafe_parent = dir.path().join("unsafe:mount"); + fs::create_dir_all(&unsafe_parent).unwrap(); + let db = unsafe_parent.join("memory.db"); + let runtime = fake_container_runtime_named(dir.path(), "docker"); + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db) + .args(["mcp", "codex", "preview", "--runtime", "container"]) + .output() + .unwrap(); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("cannot be represented safely"), "{stderr}"); + assert!(stderr.contains("--volume"), "{stderr}"); + assert!( + !codex_home.exists(), + "unsafe mount must not create CODEX_HOME" + ); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_missing_and_invalid_uid_gid_fail_before_mutation() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join("database"); + let empty_path = dir.path().join("empty-bin"); + fs::create_dir_all(&db_dir).unwrap(); + fs::create_dir_all(&empty_path).unwrap(); + let db = db_dir.join("memory.db"); + let runtime = fake_container_runtime_named(dir.path(), "docker"); + + let run = |uid: Option<&str>, gid: Option<&str>, path: Option<&std::path::Path>| { + let mut command = bin(); + clear_runtime_env(&mut command); + command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .arg("--db") + .arg(&db) + .args(["mcp", "codex", "preview", "--runtime", "container"]); + if let Some(uid) = uid { + command.env("CODEX_MEMORYD_UID", uid); + } + if let Some(gid) = gid { + command.env("CODEX_MEMORYD_GID", gid); + } + if let Some(path) = path { + command.env("PATH", path); + } + command.output().unwrap() + }; + + let missing_uid = run(None, None, Some(&empty_path)); + assert!(!missing_uid.status.success()); + let missing_uid_stderr = String::from_utf8_lossy(&missing_uid.stderr); + assert!(missing_uid_stderr.contains("CODEX_MEMORYD_UID")); + assert!(missing_uid_stderr.contains("id -u")); + + let missing_gid = run(Some("1001"), None, Some(&empty_path)); + assert!(!missing_gid.status.success()); + let missing_gid_stderr = String::from_utf8_lossy(&missing_gid.stderr); + assert!(missing_gid_stderr.contains("CODEX_MEMORYD_GID")); + assert!(missing_gid_stderr.contains("id -g")); + + let invalid_uid = run(Some("not-a-number"), Some("1002"), None); + assert!(!invalid_uid.status.success()); + assert!(String::from_utf8_lossy(&invalid_uid.stderr) + .contains("valid numeric value for CODEX_MEMORYD_UID")); + + let invalid_gid = run(Some("1001"), Some("-1"), None); + assert!(!invalid_gid.status.success()); + assert!(String::from_utf8_lossy(&invalid_gid.stderr) + .contains("valid numeric value for CODEX_MEMORYD_GID")); + assert!( + !codex_home.exists(), + "invalid IDs must not create CODEX_HOME" + ); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_preview_and_status_write_nothing() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join("database"); + fs::create_dir_all(&db_dir).unwrap(); + let db = db_dir.join("memory.db"); + let runtime = fake_container_runtime_named(dir.path(), "docker"); + + for operation in ["preview", "status"] { + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db) + .args(["mcp", "codex", operation, "--runtime", "container"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{operation}: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !codex_home.exists(), + "{operation} must not create CODEX_HOME" + ); + assert!( + !codex_home.join("config.toml").exists(), + "{operation} must not write config.toml" + ); + } +} + #[test] fn cli_mcp_codex_apply_creates_config_and_is_idempotent() { let dir = TempDir::new().unwrap(); @@ -6512,6 +7007,41 @@ fn cli_mcp_codex_apply_creates_config_and_is_idempotent() { assert!(backup_files(&codex_config).is_empty()); } +#[cfg(unix)] +#[test] +fn cli_mcp_codex_apply_only_checks_runtime_and_does_not_pull_or_start() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join("database"); + fs::create_dir_all(&db_dir).unwrap(); + let db = db_dir.join("memory.db"); + let (runtime, invocations) = fake_container_runtime_with_log(dir.path()); + + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db) + .args(["mcp", "codex", "apply", "--runtime", "container"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(fs::read_to_string(invocations).unwrap(), "--version\n"); + assert!(fs::read_to_string(&codex_config_path(&dir)) + .unwrap() + .contains("[mcp_servers.codex_memoryd]")); +} + #[test] fn cli_mcp_codex_apply_updates_owned_block_and_preserves_unrelated_config() { let dir = TempDir::new().unwrap(); @@ -6621,6 +7151,54 @@ args = ["hello"] assert!(!updated.contains("[mcp_servers.codex_memoryd]")); } +#[cfg(unix)] +#[test] +fn cli_mcp_codex_remove_works_when_previous_container_runtime_is_unavailable() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let codex_config = codex_config_path(&dir); + let missing_runtime = dir.path().join("runtime-no-longer-installed"); + let missing_db = dir.path().join("old-data").join("memory.db"); + fs::create_dir_all(&codex_home).unwrap(); + let original = r#"[mcp_servers.codex_memoryd] +command = "/old/docker" +args = ["run", "--rm", "old-image", "mcp", "stdio", "--read-only"] + +[mcp_servers.other] +command = "/bin/echo" +args = ["preserve me"] +"#; + fs::write(&codex_config, original).unwrap(); + + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &missing_runtime) + .env("CODEX_MEMORYD_DB", &missing_db) + .args(["mcp", "codex", "remove", "--runtime", "container"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let json: Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(json["status"], "removed"); + assert_eq!(json["changed"], true); + assert_eq!(json["resolved"], Value::Null); + let updated = fs::read_to_string(&codex_config).unwrap(); + assert!(!updated.contains("[mcp_servers.codex_memoryd]")); + assert!(updated.contains("[mcp_servers.other]")); + assert_eq!( + fs::read_to_string(&backup_files(&codex_config)[0]).unwrap(), + original + ); +} + #[test] fn cli_eval_retrieval_supports_subset_limit_dry_run_and_report_out() { let dir = TempDir::new().unwrap(); From fda2593737d16350c478f2aace7815025a616fa8 Mon Sep 17 00:00:00 2001 From: joshyorko Date: Thu, 27 Aug 2026 08:39:25 -0400 Subject: [PATCH 2/4] docs: add MCP onboarding guide --- README.md | 2 + docs/getting-started.md | 81 +++++++++++ ...ontainer-mcp-homebrew-onboarding-design.md | 134 ++++++++++++++++++ 3 files changed, 217 insertions(+) create mode 100644 docs/getting-started.md create mode 100644 docs/superpowers/specs/2026-08-19-container-mcp-homebrew-onboarding-design.md diff --git a/README.md b/README.md index 0c3fe14..a463be0 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,8 @@ heartbeat, adapter exports, and MCP stdio are delivery modes. Memory is recall, not authority: retrieved context can inform a turn, but it never overrides the current user, repo, or policy state. +For the shortest install-to-first-recall path, see [`docs/getting-started.md`](./docs/getting-started.md). + ## Current Surface This is the landed MVP surface today: diff --git a/docs/getting-started.md b/docs/getting-started.md new file mode 100644 index 0000000..ee3a981 --- /dev/null +++ b/docs/getting-started.md @@ -0,0 +1,81 @@ +# Getting started + +This guide connects Codex to the read-only `codex-memoryd` MCP server. The +recommended release-shaped path uses the native binary: + +```zsh +# Future release command: the formula and release artifacts are not published yet. +brew install joshyorko/tools/codex-memoryd +codex-memoryd mcp codex apply +codex-memoryd mcp codex status +``` + +Restart Codex after applying the configuration, then call `memory_status` to +verify that the server starts and responds. `preview` and `status` inspect the +owned Codex MCP block; `apply` updates only that block and backs up the existing +Codex configuration before changing it. + +## Opt in to the container launcher + +Use the container runtime when Docker or Podman should launch the MCP image: + +```zsh +codex-memoryd mcp codex preview --runtime container +codex-memoryd mcp codex apply --runtime container +codex-memoryd mcp codex status --runtime container +``` + +The CLI performs setup and does not pull an image or start a container during +`apply`. When Codex later starts the MCP server, the generated stdio command +launches the configured Docker or Podman image on demand. The default image is +`ghcr.io/joshyorko/codex-memoryd:latest`; `CODEX_MEMORYD_IMAGE` can override it. +The container mounts the parent directory of the persistent SQLite database at +`/data`, using the corresponding database filename inside the container. This +directory mount preserves SQLite sidecar files such as WAL and shared-memory +files. The database remains persistent host data; its exact path is determined +by the resolved configuration. +The selected database parent must already exist; run codex-memoryd init or +create the configured directory before previewing the container launcher. + +Restart Codex after applying the container configuration and call +`memory_status` again. Docker or Podman must be installed and available to the +CLI and to Codex when the MCP server starts. + +## Switch runtimes or remove the integration + +Switch back to the native binary with: + +```zsh +codex-memoryd mcp codex preview --runtime native +codex-memoryd mcp codex apply --runtime native +codex-memoryd mcp codex status --runtime native +``` + +Restart Codex and verify with `memory_status`. To remove only the owned +`codex-memoryd` MCP block: + +```zsh +codex-memoryd mcp codex remove +codex-memoryd mcp codex status +``` + +Removing the MCP block, uninstalling the Homebrew formula, or removing the +container image does not delete the persistent memory database. Delete that +database separately only if you intentionally want to discard stored memory. +To uninstall the future-release Homebrew installation after removing the MCP +block, run `brew uninstall codex-memoryd`. + +## Source-build fallback + +If the future-release Homebrew command is unavailable, build the binary from a +checkout and use `target/release/codex-memoryd` in the commands above: + +```zsh +cargo build --release +target/release/codex-memoryd mcp codex apply +target/release/codex-memoryd mcp codex status +``` + +Homebrew, release, image, tag, and tap artifacts are not published in this +feature slice. The Homebrew command above becomes usable only after a later +release and tap publication completes. diff --git a/docs/superpowers/specs/2026-08-19-container-mcp-homebrew-onboarding-design.md b/docs/superpowers/specs/2026-08-19-container-mcp-homebrew-onboarding-design.md new file mode 100644 index 0000000..c256645 --- /dev/null +++ b/docs/superpowers/specs/2026-08-19-container-mcp-homebrew-onboarding-design.md @@ -0,0 +1,134 @@ +# Container MCP Launcher and Homebrew Onboarding + +## Context + +`codex-memoryd mcp codex preview|apply|status|remove` manages the owned Codex +MCP block. The global `--runtime native|container` option already parses for +these commands, but MCP config rendering always emits the native binary path. +Josh's current Codex config therefore points at a native binary that no longer +exists. + +The repository also lacks a short release-user guide. The README contains +source-build, dogfood, daemon, and operator detail, but there is no minimal +Homebrew install-to-first-recall path. There are currently no repository tags, +GitHub releases, release workflow, or `homebrew-tools` formula for this binary, +so Homebrew instructions must be clearly described as the intended release +contract until those artifacts land. + +## Decision + +Make the existing global runtime selector authoritative for Codex MCP config +generation: + +- `--runtime native` preserves the current direct-binary stdio entry. +- `--runtime container` emits a Docker or Podman stdio entry that starts the + published image on demand and exits when Codex closes stdin. +- Omitting `--runtime` preserves the resolved/default native behavior. + +Do not add another runtime option under `mcp codex`. Because the root option is +global, both placements remain valid: + +```zsh +codex-memoryd --runtime container mcp codex apply +codex-memoryd mcp codex apply --runtime container +``` + +The documentation will use the second form because it reads naturally at the +point where the choice matters. + +## Generated Container Contract + +The container MCP block uses the runtime already resolved by +`codex-memoryd`: + +- command: resolved `docker` or `podman` executable +- transport: interactive stdio (`run -i`) +- lifecycle: ephemeral container (`--rm`) +- image acquisition: pull when missing (`--pull=missing`) +- image: resolved `CODEX_MEMORYD_IMAGE`, defaulting to + `ghcr.io/joshyorko/codex-memoryd:latest` +- identity: resolved host UID/GID so SQLite files remain host-owned +- storage: mount only the database's parent directory read-write at `/data` +- database argument: the corresponding `/data/` path +- MCP tier: explicit `mcp stdio --read-only` + +The existing Codex allowlist, approval mode, startup timeout, and tool timeout +remain unchanged. The database directory mount, rather than a single-file +mount, is required because SQLite may create WAL and shared-memory sidecars. + +`preview` and `status` are read-only. `apply` mutates only the owned MCP table, +backs up an existing Codex config before changing it, and remains idempotent. +It does not pull the image or start a container; Codex does that when it starts +the MCP server. + +## First-Time User Guide + +Add a short `docs/getting-started.md` and link it near the top of the README. +The release-shaped native path is: + +```zsh +brew install joshyorko/tools/codex-memoryd +codex-memoryd mcp codex apply +codex-memoryd mcp codex status +``` + +The guide then tells the user to restart Codex and call `memory_status`. +Homebrew-native is the recommendation because the installed artifact is +already available and requires no container runtime. + +The opt-in container path is: + +```zsh +codex-memoryd mcp codex preview --runtime container +codex-memoryd mcp codex apply --runtime container +codex-memoryd mcp codex status --runtime container +``` + +It explains that the Homebrew CLI performs setup while Codex subsequently +launches the MCP image on demand. The guide includes switching back to native, +uninstalling the MCP block, the persistent database location, and a concise +note that removing the MCP block or formula does not delete memory data. + +The tap command is marked as a future release command until the formula and +release artifacts exist. Source-build instructions remain available from the +README but are not part of the primary user journey. + +## Errors + +Container rendering fails before config mutation when: + +- neither Docker nor Podman can be resolved; +- the database has no usable file name or parent directory; +- UID/GID or mount values cannot be represented safely. + +Errors name the failed requirement and give the next command or configuration +override. Runtime image-pull, permission, and container-start failures remain +visible as MCP startup failures from Docker or Podman; the generated arguments +must not suppress their stderr. + +## Tests + +Focused CLI tests prove: + +- native output remains byte-for-byte compatible; +- container preview contains the resolved engine, image, identity, mount, and + in-container database path; +- Docker and Podman selections render deterministically; +- spaces and TOML-sensitive characters in host paths are escaped correctly; +- preview and status do not write; +- apply backs up and replaces a drifted block; +- repeated apply is idempotent; +- remove deletes only the owned block; +- missing container runtime and invalid database paths fail before mutation; +- CLI help continues to expose one global `--runtime` option. + +Documentation checks assert that the short guide contains native install, +container opt-in, verification, switching, and uninstall/data-retention paths. + +## Release Boundary + +This slice makes the CLI and documentation release-shaped but does not publish +an artifact, create a GitHub release, modify `homebrew-tools`, or install the +formula. A later packaging slice must produce checksummed platform artifacts, +add the formula to the tap, test installation from the tap, and replace the +guide's future-release label only after those checks pass. From b8bb09aadeb33750fdeeefd1787418a112f7ee01 Mon Sep 17 00:00:00 2001 From: joshyorko Date: Thu, 27 Aug 2026 08:47:02 -0400 Subject: [PATCH 3/4] test: cover MCP launcher validation boundaries --- src/cli.rs | 31 +++++++++++ tests/cli_smoke.rs | 133 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 163 insertions(+), 1 deletion(-) diff --git a/src/cli.rs b/src/cli.rs index decada4..f883a68 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -2652,6 +2652,11 @@ fn container_mcp_id(value: Option<&str>, variable: &str, fallback: &str) -> Resu } fn container_mcp_database(runtime: &RuntimeOptions) -> Result<(String, String)> { + if runtime.db.as_os_str().is_empty() { + return Err(error::Error::invalid_request( + "container MCP database path must name a file; set CODEX_MEMORYD_DB=/path/to/memory.db", + )); + } let database = if runtime.db.is_absolute() { runtime.db.clone() } else { @@ -3679,3 +3684,29 @@ fn render_card_markdown(card: &CardShowResponse) -> String { lines.join("\n") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn toml_basic_string_round_trips_toml_sensitive_values() { + let original = "spaces \"quotes\" \\backslashes\nand\tcontrols\u{0001}\u{007f}"; + let encoded = toml_basic_string(original, "test value").unwrap(); + assert!(encoded.contains("\\u0001")); + assert!(encoded.contains("\\u007F")); + assert!(!encoded.contains("\\u{")); + let document: toml::Value = toml::from_str(&format!("value = {encoded}\n")).unwrap(); + assert_eq!( + document.get("value").and_then(toml::Value::as_str), + Some(original) + ); + } + + #[test] + fn toml_basic_string_rejects_nul() { + let error = toml_basic_string("unsafe\0value", "test value").unwrap_err(); + assert!(error.message.contains("cannot represent")); + assert!(error.message.contains("NUL")); + } +} diff --git a/tests/cli_smoke.rs b/tests/cli_smoke.rs index fdbd222..c23a086 100644 --- a/tests/cli_smoke.rs +++ b/tests/cli_smoke.rs @@ -6655,6 +6655,54 @@ fn cli_mcp_codex_container_global_runtime_placements_and_engines_are_determinist let _: toml::Value = toml::from_str(podman_output["snippet"].as_str().unwrap()).unwrap(); } +#[cfg(unix)] +#[test] +fn cli_mcp_codex_each_action_accepts_both_global_runtime_placements() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let db_dir = dir.path().join("database"); + fs::create_dir_all(&db_dir).unwrap(); + let db = db_dir.join("memory.db"); + let runtime = fake_container_runtime_named(dir.path(), "docker"); + + for action in ["preview", "apply", "status", "remove"] { + let codex_home = dir.path().join(format!("codex-home-{action}")); + let codex_config = codex_home.join("config.toml"); + if action != "preview" { + fs::create_dir_all(&codex_home).unwrap(); + fs::write( + &codex_config, + "[mcp_servers.codex_memoryd]\ncommand = \"/old\"\n", + ) + .unwrap(); + } + + for before_subcommand in [true, false] { + let mut command = bin(); + clear_runtime_env(&mut command); + command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db); + if before_subcommand { + command.args(["--runtime", "container", "mcp", "codex", action]); + } else { + command.args(["mcp", "codex", action, "--runtime", "container"]); + } + let output = command.output().unwrap(); + assert!( + output.status.success(), + "{action} before={before_subcommand}: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + } +} + #[cfg(unix)] #[test] fn cli_mcp_codex_container_toml_round_trips_spaces_quotes_backslashes_and_image_values() { @@ -6859,6 +6907,65 @@ fn cli_mcp_codex_container_unsafe_mount_path_fails_before_config_mutation() { ); } +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_unusable_database_filename_fails_before_mutation() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join("database"); + fs::create_dir_all(&db_dir).unwrap(); + let db_without_filename = PathBuf::from("/"); + let runtime = fake_container_runtime_named(dir.path(), "docker"); + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db_without_filename) + .args(["mcp", "codex", "preview", "--runtime", "container"]) + .output() + .unwrap(); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("database path must name"), "{stderr}"); + assert!(stderr.contains("CODEX_MEMORYD_DB"), "{stderr}"); + assert!(!codex_home.exists()); +} + +#[cfg(unix)] +#[test] +fn cli_mcp_codex_container_control_character_filename_fails_before_mutation() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let db_dir = dir.path().join("database"); + fs::create_dir_all(&db_dir).unwrap(); + let db = db_dir.join("memory\n.db"); + let runtime = fake_container_runtime_named(dir.path(), "docker"); + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .env("CODEX_MEMORYD_CONTAINER_RUNTIME", &runtime) + .env("CODEX_MEMORYD_UID", "1001") + .env("CODEX_MEMORYD_GID", "1002") + .arg("--db") + .arg(&db) + .args(["mcp", "codex", "apply", "--runtime", "container"]) + .output() + .unwrap(); + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("control characters"), "{stderr}"); + assert!(!codex_home.exists()); +} + #[cfg(unix)] #[test] fn cli_mcp_codex_container_missing_and_invalid_uid_gid_fail_before_mutation() { @@ -6962,6 +7069,30 @@ fn cli_mcp_codex_preview_and_status_write_nothing() { } } +#[test] +fn cli_mcp_codex_native_status_writes_nothing() { + let dir = TempDir::new().unwrap(); + let memoryd_home = dir.path().join("memoryd-home"); + let codex_home = codex_home_path(&dir); + let mut command = bin(); + clear_runtime_env(&mut command); + let output = command + .env("CODEX_MEMORYD_HOME", &memoryd_home) + .env("CODEX_HOME", &codex_home) + .args(["--runtime", "native", "mcp", "codex", "status"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !codex_home.exists(), + "native status must not create CODEX_HOME" + ); +} + #[test] fn cli_mcp_codex_apply_creates_config_and_is_idempotent() { let dir = TempDir::new().unwrap(); @@ -7037,7 +7168,7 @@ fn cli_mcp_codex_apply_only_checks_runtime_and_does_not_pull_or_start() { String::from_utf8_lossy(&output.stderr) ); assert_eq!(fs::read_to_string(invocations).unwrap(), "--version\n"); - assert!(fs::read_to_string(&codex_config_path(&dir)) + assert!(fs::read_to_string(codex_config_path(&dir)) .unwrap() .contains("[mcp_servers.codex_memoryd]")); } From 7756c947465dd8765f567399afc4fb9d55a56132 Mon Sep 17 00:00:00 2001 From: Josh Yorko <54248591+joshyorko@users.noreply.github.com> Date: Thu, 27 Aug 2026 19:24:04 -0400 Subject: [PATCH 4/4] fix: preserve container MCP scope and identity --- src/cli.rs | 40 ++++++++++++++++++++++++++-------------- tests/cli_smoke.rs | 22 ++++++++++++++++++---- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index f883a68..34f401c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -2590,33 +2590,30 @@ fn codex_mcp_resolved(runtime: &RuntimeOptions) -> Result { )) })?; let (database_parent, database_filename) = container_mcp_database(runtime)?; - let uid = container_mcp_id( - runtime.uid.as_deref(), - "CODEX_MEMORYD_UID", - "id -u", - )?; - let gid = container_mcp_id( - runtime.gid.as_deref(), - "CODEX_MEMORYD_GID", - "id -g", - )?; - let args = vec![ + let mut args = vec![ "run".to_string(), "--rm".to_string(), "-i".to_string(), "--pull=missing".to_string(), - "--user".to_string(), - format!("{uid}:{gid}"), + ]; + args.extend(container_mcp_identity_args(&command, runtime)?); + args.extend([ "--volume".to_string(), format!("{database_parent}:/data"), + "--env".to_string(), + format!("CODEX_MEMORYD_PROFILE={}", runtime.profile), + "--env".to_string(), + format!("CODEX_MEMORYD_WORKSPACE={}", runtime.workspace), + "--env".to_string(), + format!("CODEX_MEMORYD_LOG={}", runtime.log_level), runtime.image.clone(), "--db".to_string(), format!("/data/{database_filename}"), "mcp".to_string(), "stdio".to_string(), "--read-only".to_string(), - ]; + ]); Ok(CodexMcpResolved { command, @@ -2630,6 +2627,21 @@ fn codex_mcp_resolved(runtime: &RuntimeOptions) -> Result { } } +fn container_mcp_identity_args(command: &str, runtime: &RuntimeOptions) -> Result> { + let engine = std::path::Path::new(command) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(command) + .trim_end_matches(".exe"); + if engine.eq_ignore_ascii_case("podman") { + return Ok(vec!["--userns=keep-id".to_string()]); + } + + let uid = container_mcp_id(runtime.uid.as_deref(), "CODEX_MEMORYD_UID", "id -u")?; + let gid = container_mcp_id(runtime.gid.as_deref(), "CODEX_MEMORYD_GID", "id -g")?; + Ok(vec!["--user".to_string(), format!("{uid}:{gid}")]) +} + fn container_mcp_id(value: Option<&str>, variable: &str, fallback: &str) -> Result { let Some(value) = value else { return Err(error::Error::invalid_request(format!( diff --git a/tests/cli_smoke.rs b/tests/cli_smoke.rs index c23a086..1001263 100644 --- a/tests/cli_smoke.rs +++ b/tests/cli_smoke.rs @@ -55,6 +55,9 @@ fn clear_runtime_env(command: &mut Command) { "CODEX_MEMORYD_IMAGE", "CODEX_MEMORYD_UID", "CODEX_MEMORYD_GID", + "CODEX_MEMORYD_PROFILE", + "CODEX_MEMORYD_WORKSPACE", + "CODEX_MEMORYD_LOG", ] { command.env_remove(key); } @@ -6572,6 +6575,12 @@ fn cli_mcp_codex_container_global_runtime_placements_and_engines_are_determinist let codex_home = codex_home_path(&dir); let db_dir = dir.path().join("database"); fs::create_dir_all(&db_dir).unwrap(); + fs::create_dir_all(&memoryd_home).unwrap(); + fs::write( + memoryd_home.join("runtime.env"), + "CODEX_MEMORYD_PROFILE=work\nCODEX_MEMORYD_WORKSPACE=launcher-test\nCODEX_MEMORYD_LOG=debug\n", + ) + .unwrap(); let db = db_dir.join("memory.db"); let docker = fake_container_runtime_named(dir.path(), "docker"); let podman = fake_container_runtime_named(dir.path(), "podman"); @@ -6615,6 +6624,12 @@ fn cli_mcp_codex_container_global_runtime_placements_and_engines_are_determinist "1234:5678", "--volume", format!("{}:/data", db_dir.canonicalize().unwrap().display()), + "--env", + "CODEX_MEMORYD_PROFILE=work", + "--env", + "CODEX_MEMORYD_WORKSPACE=launcher-test", + "--env", + "CODEX_MEMORYD_LOG=debug", "ghcr.io/example/codex-memoryd:test", "--db", "/data/memory.db", @@ -6640,10 +6655,9 @@ fn cli_mcp_codex_container_global_runtime_placements_and_engines_are_determinist podman_output["resolved"]["command"], podman.to_string_lossy().as_ref() ); - assert_eq!( - podman_output["resolved"]["args"], - docker_before["resolved"]["args"] - ); + let podman_args = podman_output["resolved"]["args"].as_array().unwrap(); + assert!(podman_args.iter().any(|arg| arg == "--userns=keep-id")); + assert!(!podman_args.iter().any(|arg| arg == "--user")); assert_ne!(podman_output["snippet"], docker_before["snippet"]); assert!(podman_output["snippet"] .as_str()