From 8c1e3f3e6470a1b4cb5d8283e561ce55705394c5 Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Fri, 27 Feb 2026 17:42:25 -0800 Subject: [PATCH 1/3] app-server: Add `ephemeral` field to `Thread` object (#13084) Currently there is no alternative way to know that thread is ephemeral, only client which did create it has the knowledge. --- .../schema/json/ServerNotification.json | 5 ++++ .../codex_app_server_protocol.schemas.json | 5 ++++ .../schema/json/v2/ThreadForkResponse.json | 5 ++++ .../schema/json/v2/ThreadListResponse.json | 5 ++++ .../schema/json/v2/ThreadReadResponse.json | 5 ++++ .../schema/json/v2/ThreadResumeResponse.json | 5 ++++ .../json/v2/ThreadRollbackResponse.json | 5 ++++ .../schema/json/v2/ThreadStartResponse.json | 5 ++++ .../json/v2/ThreadStartedNotification.json | 5 ++++ .../json/v2/ThreadUnarchiveResponse.json | 5 ++++ .../schema/typescript/v2/Thread.ts | 4 +++ .../app-server-protocol/src/protocol/v2.rs | 2 ++ codex-rs/app-server/README.md | 1 + .../app-server/src/codex_message_processor.rs | 2 ++ codex-rs/app-server/src/thread_status.rs | 1 + .../app-server/tests/suite/v2/thread_read.rs | 16 ++++++++++ .../app-server/tests/suite/v2/thread_start.rs | 30 +++++++++++++++++++ codex-rs/core/src/codex.rs | 1 + codex-rs/core/src/codex_thread.rs | 1 + 19 files changed, 108 insertions(+) diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 81797e282f..7a29ef92b6 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1655,6 +1655,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -1724,6 +1728,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index a9fc41aed7..374d43de7f 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -12498,6 +12498,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -12567,6 +12571,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index c1dee7a589..fd38876505 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -882,6 +882,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -951,6 +955,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json index e9bce99fbd..2cf3b940f3 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json @@ -655,6 +655,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -724,6 +728,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json index e487ad0d8d..b2398692fd 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json @@ -655,6 +655,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -724,6 +728,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index b606e34a28..56e268bb53 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -882,6 +882,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -951,6 +955,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json index faea619708..4ac54973de 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json @@ -655,6 +655,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -724,6 +728,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index a1de9f9efa..043354b68a 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -882,6 +882,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -951,6 +955,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json index f8faff0101..c46d4d7bb0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json @@ -655,6 +655,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -724,6 +728,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json index b1be7b2de6..eabd614028 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json @@ -655,6 +655,10 @@ "description": "Working directory captured for the thread.", "type": "string" }, + "ephemeral": { + "description": "Whether the thread is ephemeral and should not be materialized on disk.", + "type": "boolean" + }, "gitInfo": { "anyOf": [ { @@ -724,6 +728,7 @@ "cliVersion", "createdAt", "cwd", + "ephemeral", "id", "modelProvider", "preview", diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts b/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts index ee07081dca..56c5688495 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts @@ -11,6 +11,10 @@ export type Thread = { id: string, * Usually the first user message in the thread, if available. */ preview: string, +/** + * Whether the thread is ephemeral and should not be materialized on disk. + */ +ephemeral: boolean, /** * Model provider used for this thread (for example, 'openai'). */ diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index c3d2cef29b..a26ca25bd5 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -2517,6 +2517,8 @@ pub struct Thread { pub id: String, /// Usually the first user message in the thread, if available. pub preview: String, + /// Whether the thread is ephemeral and should not be materialized on disk. + pub ephemeral: bool, /// Model provider used for this thread (for example, 'openai'). pub model_provider: String, /// Unix timestamp (in seconds) when the thread was created. diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index b7341801de..b43cd99c6a 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -62,6 +62,7 @@ Use the thread APIs to create, list, or archive conversations. Drive a conversat - Initialize once per connection: Immediately after opening a transport connection, send an `initialize` request with your client metadata, then emit an `initialized` notification. Any other request on that connection before this handshake gets rejected. - Start (or resume) a thread: Call `thread/start` to open a fresh conversation. The response returns the thread object and you’ll also get a `thread/started` notification. If you’re continuing an existing conversation, call `thread/resume` with its ID instead. If you want to branch from an existing conversation, call `thread/fork` to create a new thread id with copied history. + The returned `thread.ephemeral` flag tells you whether the session is intentionally in-memory only; when it is `true`, `thread.path` is `null`. - Begin a turn: To send user input, call `turn/start` with the target `threadId` and the user's input. Optional fields let you override model, cwd, sandbox policy, etc. This immediately returns the new turn object and triggers a `turn/started` notification. - Stream events: After `turn/start`, keep reading JSON-RPC notifications on stdout. You’ll see `item/started`, `item/completed`, deltas like `item/agentMessage/delta`, tool progress, etc. These represent streaming model output plus any side effects (commands, tool calls, reasoning notes). - Finish the turn: When the model is done (or the turn is interrupted via making the `turn/interrupt` call), the server sends `turn/completed` with the final turn state and token usage. diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c4532cafd9..674dea42fb 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -7869,6 +7869,7 @@ fn build_thread_from_snapshot( Thread { id: thread_id.to_string(), preview: String::new(), + ephemeral: config_snapshot.ephemeral, model_provider: config_snapshot.model_provider_id.clone(), created_at: now, updated_at: now, @@ -7910,6 +7911,7 @@ pub(crate) fn summary_to_thread(summary: ConversationSummary) -> Thread { Thread { id: conversation_id.to_string(), preview, + ephemeral: false, model_provider, created_at: created_at.map(|dt| dt.timestamp()).unwrap_or(0), updated_at: updated_at.map(|dt| dt.timestamp()).unwrap_or(0), diff --git a/codex-rs/app-server/src/thread_status.rs b/codex-rs/app-server/src/thread_status.rs index f3a7c1fd83..c5faa42d16 100644 --- a/codex-rs/app-server/src/thread_status.rs +++ b/codex-rs/app-server/src/thread_status.rs @@ -733,6 +733,7 @@ mod tests { Thread { id: thread_id.to_string(), preview: String::new(), + ephemeral: false, model_provider: "mock-provider".to_string(), created_at: 0, updated_at: 0, diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index 5d2211d3c8..196f128c46 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -78,6 +78,7 @@ async fn thread_read_returns_summary_without_turns() -> Result<()> { assert_eq!(thread.id, conversation_id); assert_eq!(thread.preview, preview); assert_eq!(thread.model_provider, "mock_provider"); + assert!(!thread.ephemeral, "stored rollouts should not be ephemeral"); assert!(thread.path.as_ref().expect("thread path").is_absolute()); assert_eq!(thread.cwd, PathBuf::from("/")); assert_eq!(thread.cli_version, "0.0.0"); @@ -278,6 +279,11 @@ async fn thread_name_set_is_reflected_in_read_list_and_resume() -> Result<()> { Some(new_name), "thread/read must serialize `thread.name` on the wire" ); + assert_eq!( + thread_json.get("ephemeral").and_then(Value::as_bool), + Some(false), + "thread/read must serialize `thread.ephemeral` on the wire" + ); // List should also surface the name. let list_id = mcp @@ -317,6 +323,11 @@ async fn thread_name_set_is_reflected_in_read_list_and_resume() -> Result<()> { Some(new_name), "thread/list must serialize `thread.name` on the wire" ); + assert_eq!( + listed_json.get("ephemeral").and_then(Value::as_bool), + Some(false), + "thread/list must serialize `thread.ephemeral` on the wire" + ); // Resume should also surface the name. let resume_id = mcp @@ -345,6 +356,11 @@ async fn thread_name_set_is_reflected_in_read_list_and_resume() -> Result<()> { Some(new_name), "thread/resume must serialize `thread.name` on the wire" ); + assert_eq!( + resumed_json.get("ephemeral").and_then(Value::as_bool), + Some(false), + "thread/resume must serialize `thread.ephemeral` on the wire" + ); Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index c0b84fe42f..7871f58351 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -62,6 +62,10 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> { thread.created_at > 0, "created_at should be a positive UNIX timestamp" ); + assert!( + !thread.ephemeral, + "new persistent threads should not be ephemeral" + ); assert_eq!(thread.status, ThreadStatus::Idle); let thread_path = thread.path.clone().expect("thread path should be present"); assert!(thread_path.is_absolute(), "thread path should be absolute"); @@ -80,6 +84,11 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> { Some(&Value::Null), "new threads should serialize `name: null`" ); + assert_eq!( + thread_json.get("ephemeral").and_then(Value::as_bool), + Some(false), + "new persistent threads should serialize `ephemeral: false`" + ); assert_eq!(thread.name, None); // A corresponding thread/started notification should arrive. @@ -98,6 +107,13 @@ async fn thread_start_creates_thread_and_emits_started() -> Result<()> { Some(&Value::Null), "thread/started should serialize `name: null` for new threads" ); + assert_eq!( + started_thread_json + .get("ephemeral") + .and_then(Value::as_bool), + Some(false), + "thread/started should serialize `ephemeral: false` for new persistent threads" + ); let started: ThreadStartedNotification = serde_json::from_value(notif.params.expect("params must be present"))?; assert_eq!(started.thread, thread); @@ -196,11 +212,25 @@ async fn thread_start_ephemeral_remains_pathless() -> Result<()> { mcp.read_stream_until_response_message(RequestId::Integer(req_id)), ) .await??; + let resp_result = resp.result.clone(); let ThreadStartResponse { thread, .. } = to_response::(resp)?; + assert!( + thread.ephemeral, + "ephemeral threads should be marked explicitly" + ); assert_eq!( thread.path, None, "ephemeral threads should not expose a path" ); + let thread_json = resp_result + .get("thread") + .and_then(Value::as_object) + .expect("thread/start result.thread must be an object"); + assert_eq!( + thread_json.get("ephemeral").and_then(Value::as_bool), + Some(true), + "ephemeral threads should serialize `ephemeral: true`" + ); Ok(()) } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 885554c4ca..38dde09a08 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -851,6 +851,7 @@ impl SessionConfiguration { approval_policy: self.approval_policy.value(), sandbox_policy: self.sandbox_policy.get().clone(), cwd: self.cwd.clone(), + ephemeral: self.original_config_do_not_use.ephemeral, reasoning_effort: self.collaboration_mode.reasoning_effort(), personality: self.personality, session_source: self.session_source.clone(), diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 19a8214ee0..dc3c4ea753 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -30,6 +30,7 @@ pub struct ThreadConfigSnapshot { pub approval_policy: AskForApproval, pub sandbox_policy: SandboxPolicy, pub cwd: PathBuf, + pub ephemeral: bool, pub reasoning_effort: Option, pub personality: Option, pub session_source: SessionSource, From 033ef9cb9db1cd89a947ce44e2af425e60f8cf84 Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Fri, 27 Feb 2026 17:45:55 -0800 Subject: [PATCH 2/3] feat: add debug clear-memories command to hard-wipe memories state (#13085) #### what adds a `codex debug clear-memories` command to help with clearing all memories state from disk, sqlite db, and marking threads as `memory_mode=disabled` so they don't get resummarized when the `memories` feature is re-enabled. #### tests add tests --- codex-rs/Cargo.lock | 2 + codex-rs/cli/Cargo.toml | 2 + codex-rs/cli/src/main.rs | 63 +++++++++ codex-rs/cli/tests/debug_clear_memories.rs | 141 +++++++++++++++++++++ codex-rs/state/src/runtime/memories.rs | 136 ++++++++++++++++++++ 5 files changed, 344 insertions(+) create mode 100644 codex-rs/cli/tests/debug_clear_memories.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 8398d0b99c..2ab1d5943c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1603,6 +1603,7 @@ dependencies = [ "codex-protocol", "codex-responses-api-proxy", "codex-rmcp-client", + "codex-state", "codex-stdio-to-uds", "codex-tui", "codex-utils-cargo-bin", @@ -1614,6 +1615,7 @@ dependencies = [ "pretty_assertions", "regex-lite", "serde_json", + "sqlx", "supports-color 3.0.2", "tempfile", "tokio", diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index 1d955bbaaf..b197cc3d59 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -35,6 +35,7 @@ codex-mcp-server = { workspace = true } codex-protocol = { workspace = true } codex-responses-api-proxy = { workspace = true } codex-rmcp-client = { workspace = true } +codex-state = { workspace = true } codex-stdio-to-uds = { workspace = true } codex-tui = { workspace = true } libc = { workspace = true } @@ -62,3 +63,4 @@ assert_matches = { workspace = true } codex-utils-cargo-bin = { workspace = true } predicates = { workspace = true } pretty_assertions = { workspace = true } +sqlx = { workspace = true } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index c271fe0b9e..f818b743e9 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -22,6 +22,8 @@ use codex_exec::Command as ExecCommand; use codex_exec::ReviewArgs; use codex_execpolicy::ExecPolicyCheckCommand; use codex_responses_api_proxy::Args as ResponsesApiProxyArgs; +use codex_state::StateRuntime; +use codex_state::state_db_path; use codex_tui::AppExitInfo; use codex_tui::Cli as TuiCli; use codex_tui::ExitReason; @@ -163,6 +165,10 @@ struct DebugCommand { enum DebugSubcommand { /// Tooling: helps debug the app server. AppServer(DebugAppServerCommand), + + /// Internal: reset local memory state for a fresh start. + #[clap(hide = true)] + ClearMemories, } #[derive(Debug, Parser)] @@ -751,6 +757,9 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { DebugSubcommand::AppServer(cmd) => { run_debug_app_server_command(cmd)?; } + DebugSubcommand::ClearMemories => { + run_debug_clear_memories_command(&root_config_overrides, &interactive).await?; + } }, Some(Subcommand::Execpolicy(ExecpolicyCommand { sub })) => match sub { ExecpolicySubcommand::Check(cmd) => run_execpolicycheck(cmd)?, @@ -877,6 +886,60 @@ fn maybe_print_under_development_feature_warning( ); } +async fn run_debug_clear_memories_command( + root_config_overrides: &CliConfigOverrides, + interactive: &TuiCli, +) -> anyhow::Result<()> { + let cli_kv_overrides = root_config_overrides + .parse_overrides() + .map_err(anyhow::Error::msg)?; + let overrides = ConfigOverrides { + config_profile: interactive.config_profile.clone(), + ..Default::default() + }; + let config = + Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await?; + + let state_path = state_db_path(config.sqlite_home.as_path()); + let mut cleared_state_db = false; + if tokio::fs::try_exists(&state_path).await? { + let state_db = StateRuntime::init( + config.sqlite_home.clone(), + config.model_provider_id.clone(), + None, + ) + .await?; + state_db.reset_memory_data_for_fresh_start().await?; + cleared_state_db = true; + } + + let memory_root = config.codex_home.join("memories"); + let removed_memory_root = match tokio::fs::remove_dir_all(&memory_root).await { + Ok(()) => true, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => false, + Err(err) => return Err(err.into()), + }; + + let mut message = if cleared_state_db { + format!("Cleared memory state from {}.", state_path.display()) + } else { + format!("No state db found at {}.", state_path.display()) + }; + + if removed_memory_root { + message.push_str(&format!(" Removed {}.", memory_root.display())); + } else { + message.push_str(&format!( + " No memory directory found at {}.", + memory_root.display() + )); + } + + println!("{message}"); + + Ok(()) +} + /// Prepend root-level overrides so they have lower precedence than /// CLI-specific ones specified after the subcommand (if any). fn prepend_config_flags( diff --git a/codex-rs/cli/tests/debug_clear_memories.rs b/codex-rs/cli/tests/debug_clear_memories.rs new file mode 100644 index 0000000000..d8db1ebbc4 --- /dev/null +++ b/codex-rs/cli/tests/debug_clear_memories.rs @@ -0,0 +1,141 @@ +use std::path::Path; + +use anyhow::Result; +use codex_state::StateRuntime; +use codex_state::state_db_path; +use predicates::str::contains; +use sqlx::SqlitePool; +use tempfile::TempDir; + +fn codex_command(codex_home: &Path) -> Result { + let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); + cmd.env("CODEX_HOME", codex_home); + Ok(cmd) +} + +#[tokio::test] +async fn debug_clear_memories_resets_state_and_removes_memory_dir() -> Result<()> { + let codex_home = TempDir::new()?; + let runtime = StateRuntime::init( + codex_home.path().to_path_buf(), + "test-provider".to_string(), + None, + ) + .await?; + drop(runtime); + + let thread_id = "00000000-0000-0000-0000-000000000123"; + let db_path = state_db_path(codex_home.path()); + let pool = SqlitePool::connect(&format!("sqlite://{}", db_path.display())).await?; + + sqlx::query( + r#" +INSERT INTO threads ( + id, + rollout_path, + created_at, + updated_at, + source, + agent_nickname, + agent_role, + model_provider, + cwd, + cli_version, + title, + sandbox_policy, + approval_mode, + tokens_used, + first_user_message, + archived, + archived_at, + git_sha, + git_branch, + git_origin_url, + memory_mode +) VALUES (?, ?, 1, 1, 'cli', NULL, NULL, 'test-provider', ?, '', '', 'read-only', 'on-request', 0, '', 0, NULL, NULL, NULL, NULL, 'enabled') + "#, + ) + .bind(thread_id) + .bind(codex_home.path().join("session.jsonl").display().to_string()) + .bind(codex_home.path().display().to_string()) + .execute(&pool) + .await?; + + sqlx::query( + r#" +INSERT INTO stage1_outputs ( + thread_id, + source_updated_at, + raw_memory, + rollout_summary, + generated_at, + rollout_slug, + usage_count, + last_usage, + selected_for_phase2, + selected_for_phase2_source_updated_at +) VALUES (?, 1, 'raw', 'summary', 1, NULL, 0, NULL, 0, NULL) + "#, + ) + .bind(thread_id) + .execute(&pool) + .await?; + + sqlx::query( + r#" +INSERT INTO jobs ( + kind, + job_key, + status, + worker_id, + ownership_token, + started_at, + finished_at, + lease_until, + retry_at, + retry_remaining, + last_error, + input_watermark, + last_success_watermark +) VALUES + ('memory_stage1', ?, 'completed', NULL, NULL, NULL, NULL, NULL, NULL, 3, NULL, NULL, 1), + ('memory_consolidate_global', 'global', 'completed', NULL, NULL, NULL, NULL, NULL, NULL, 3, NULL, NULL, 1) + "#, + ) + .bind(thread_id) + .execute(&pool) + .await?; + + let memory_root = codex_home.path().join("memories"); + std::fs::create_dir_all(&memory_root)?; + std::fs::write(memory_root.join("memory_summary.md"), "stale memory")?; + drop(pool); + + let mut cmd = codex_command(codex_home.path())?; + cmd.args(["debug", "clear-memories"]) + .assert() + .success() + .stdout(contains("Cleared memory state")); + + let pool = SqlitePool::connect(&format!("sqlite://{}", db_path.display())).await?; + let stage1_outputs_count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM stage1_outputs") + .fetch_one(&pool) + .await?; + assert_eq!(stage1_outputs_count, 0); + + let memory_jobs_count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM jobs WHERE kind = 'memory_stage1' OR kind = 'memory_consolidate_global'", + ) + .fetch_one(&pool) + .await?; + assert_eq!(memory_jobs_count, 0); + + let memory_mode: String = sqlx::query_scalar("SELECT memory_mode FROM threads WHERE id = ?") + .bind(thread_id) + .fetch_one(&pool) + .await?; + assert_eq!(memory_mode, "disabled"); + assert!(!memory_root.exists()); + + Ok(()) +} diff --git a/codex-rs/state/src/runtime/memories.rs b/codex-rs/state/src/runtime/memories.rs index 908919d0aa..a36f9e5aa5 100644 --- a/codex-rs/state/src/runtime/memories.rs +++ b/codex-rs/state/src/runtime/memories.rs @@ -30,6 +30,19 @@ impl StateRuntime { /// stage-1 (`memory_stage1`) and phase-2 (`memory_consolidate_global`) /// memory pipelines. pub async fn clear_memory_data(&self) -> anyhow::Result<()> { + self.clear_memory_data_inner(false).await + } + + /// Resets persisted memory state for a clean-slate local start. + /// + /// In addition to clearing persisted stage-1 outputs and memory pipeline + /// jobs, this disables memory generation for all existing threads so + /// historical rollouts are not immediately picked up again. + pub async fn reset_memory_data_for_fresh_start(&self) -> anyhow::Result<()> { + self.clear_memory_data_inner(true).await + } + + async fn clear_memory_data_inner(&self, disable_existing_threads: bool) -> anyhow::Result<()> { let mut tx = self.pool.begin().await?; sqlx::query( @@ -51,6 +64,18 @@ WHERE kind = ? OR kind = ? .execute(&mut *tx) .await?; + if disable_existing_threads { + sqlx::query( + r#" +UPDATE threads +SET memory_mode = 'disabled' +WHERE memory_mode = 'enabled' + "#, + ) + .execute(&mut *tx) + .await?; + } + tx.commit().await?; Ok(()) } @@ -1158,6 +1183,8 @@ ON CONFLICT(kind, job_key) DO UPDATE SET #[cfg(test)] mod tests { + use super::JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL; + use super::JOB_KIND_MEMORY_STAGE1; use super::StateRuntime; use super::test_support::test_thread_metadata; use super::test_support::unique_temp_dir; @@ -1664,6 +1691,115 @@ mod tests { let _ = tokio::fs::remove_dir_all(codex_home).await; } + #[tokio::test] + async fn reset_memory_data_for_fresh_start_clears_rows_and_disables_threads() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + let now = Utc::now() - Duration::hours(13); + let worker_id = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("worker id"); + let enabled_thread_id = + ThreadId::from_string(&Uuid::new_v4().to_string()).expect("enabled thread id"); + let disabled_thread_id = + ThreadId::from_string(&Uuid::new_v4().to_string()).expect("disabled thread id"); + + let mut enabled = + test_thread_metadata(&codex_home, enabled_thread_id, codex_home.join("enabled")); + enabled.created_at = now; + enabled.updated_at = now; + runtime + .upsert_thread(&enabled) + .await + .expect("upsert enabled thread"); + + let claim = runtime + .try_claim_stage1_job( + enabled_thread_id, + worker_id, + enabled.updated_at.timestamp(), + 3600, + 64, + ) + .await + .expect("claim enabled thread"); + let ownership_token = match claim { + Stage1JobClaimOutcome::Claimed { ownership_token } => ownership_token, + other => panic!("unexpected claim outcome: {other:?}"), + }; + assert!( + runtime + .mark_stage1_job_succeeded( + enabled_thread_id, + ownership_token.as_str(), + enabled.updated_at.timestamp(), + "raw", + "summary", + None, + ) + .await + .expect("mark enabled thread succeeded"), + "stage1 success should be recorded" + ); + runtime + .enqueue_global_consolidation(enabled.updated_at.timestamp()) + .await + .expect("enqueue global consolidation"); + + let mut disabled = + test_thread_metadata(&codex_home, disabled_thread_id, codex_home.join("disabled")); + disabled.created_at = now; + disabled.updated_at = now; + runtime + .upsert_thread(&disabled) + .await + .expect("upsert disabled thread"); + sqlx::query("UPDATE threads SET memory_mode = 'disabled' WHERE id = ?") + .bind(disabled_thread_id.to_string()) + .execute(runtime.pool.as_ref()) + .await + .expect("disable existing thread"); + + runtime + .reset_memory_data_for_fresh_start() + .await + .expect("reset memory data"); + + let stage1_outputs_count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM stage1_outputs") + .fetch_one(runtime.pool.as_ref()) + .await + .expect("count stage1 outputs"); + assert_eq!(stage1_outputs_count, 0); + + let memory_jobs_count: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM jobs WHERE kind = ? OR kind = ?") + .bind(JOB_KIND_MEMORY_STAGE1) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .fetch_one(runtime.pool.as_ref()) + .await + .expect("count memory jobs"); + assert_eq!(memory_jobs_count, 0); + + let enabled_memory_mode: String = + sqlx::query_scalar("SELECT memory_mode FROM threads WHERE id = ?") + .bind(enabled_thread_id.to_string()) + .fetch_one(runtime.pool.as_ref()) + .await + .expect("read enabled thread memory mode"); + assert_eq!(enabled_memory_mode, "disabled"); + + let disabled_memory_mode: String = + sqlx::query_scalar("SELECT memory_mode FROM threads WHERE id = ?") + .bind(disabled_thread_id.to_string()) + .fetch_one(runtime.pool.as_ref()) + .await + .expect("read disabled thread memory mode"); + assert_eq!(disabled_memory_mode, "disabled"); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + #[tokio::test] async fn claim_stage1_jobs_enforces_global_running_cap() { let codex_home = unique_temp_dir(); From 615da1b670dfbe6409356d779e3d9066642d0d07 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 27 Feb 2026 17:54:37 -0800 Subject: [PATCH 3/3] fix: merge skill permissions into escalated sandbox by default --- codex-rs/core/src/sandboxing/mod.rs | 18 +- codex-rs/core/src/skills/loader.rs | 102 +++++++++- codex-rs/core/src/skills/permissions.rs | 158 ++++++++++++++- .../tools/runtimes/shell/unix_escalation.rs | 59 +++++- codex-rs/core/tests/suite/skill_approval.rs | 191 +++++++++++++++++- .../skill-creator/references/openai_yaml.md | 13 ++ 6 files changed, 509 insertions(+), 32 deletions(-) diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index bbd73749e4..e041734412 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -189,17 +189,17 @@ fn merge_read_only_access_with_additional_reads( } } -fn sandbox_policy_with_additional_permissions( +pub(crate) fn sandbox_policy_with_additional_permissions( sandbox_policy: &SandboxPolicy, additional_permissions: &PermissionProfile, -) -> Result { +) -> SandboxPolicy { if additional_permissions.is_empty() { - return Ok(sandbox_policy.clone()); + return sandbox_policy.clone(); } let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); - let policy = match sandbox_policy { + match sandbox_policy { SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { sandbox_policy.clone() } @@ -218,7 +218,7 @@ fn sandbox_policy_with_additional_permissions( read_only_access, extra_reads, ), - network_access: *network_access, + network_access: *network_access || additional_permissions.network.unwrap_or(false), exclude_tmpdir_env_var: *exclude_tmpdir_env_var, exclude_slash_tmp: *exclude_slash_tmp, } @@ -238,15 +238,13 @@ fn sandbox_policy_with_additional_permissions( access, extra_reads, ), - network_access: false, + network_access: additional_permissions.network.unwrap_or(false), exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } } } - }; - - Ok(policy) + } } #[derive(Default)] @@ -312,7 +310,7 @@ impl SandboxManager { } = request; let effective_policy = if let Some(additional_permissions) = spec.additional_permissions.take() { - sandbox_policy_with_additional_permissions(policy, &additional_permissions)? + sandbox_policy_with_additional_permissions(policy, &additional_permissions) } else { policy.clone() }; diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 83db7a9078..d43785e9f7 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -13,6 +13,7 @@ use crate::skills::model::SkillMetadata; use crate::skills::model::SkillPolicy; use crate::skills::model::SkillToolDependency; use crate::skills::permissions::compile_permission_profile; +use crate::skills::permissions::normalize_permission_profile; use crate::skills::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::models::PermissionProfile; @@ -55,7 +56,23 @@ struct SkillMetadataFile { #[serde(default)] policy: Option, #[serde(default)] - permissions: Option, + permissions: Option, +} + +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(rename_all = "snake_case")] +enum SkillPermissionsMode { + #[default] + Merge, + Exact, +} + +#[derive(Debug, Clone, Default, Deserialize)] +struct SkillPermissionsConfig { + #[serde(default)] + mode: SkillPermissionsMode, + #[serde(flatten)] + profile: PermissionProfile, } #[derive(Default)] @@ -595,14 +612,24 @@ fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata { policy, permissions, } = parsed; - let permission_profile = permissions.clone().filter(|profile| !profile.is_empty()); + let permission_profile = permissions + .as_ref() + .map(|permissions| normalize_permission_profile(permissions.profile.clone())) + .filter(|profile| !profile.is_empty()); + let exact_permissions = permissions.and_then(|permissions| { + if matches!(permissions.mode, SkillPermissionsMode::Exact) { + compile_permission_profile(skill_dir, Some(permissions.profile)) + } else { + None + } + }); LoadedSkillMetadata { interface: resolve_interface(interface, skill_dir), dependencies: resolve_dependencies(dependencies), policy: resolve_policy(policy), permission_profile, - permissions: compile_permission_profile(skill_dir, permissions), + permissions: exact_permissions, } } @@ -1392,6 +1419,66 @@ permissions: macos: None, }) ); + assert_eq!(outcome.skills[0].permissions, None); + } + + #[tokio::test] + async fn empty_skill_permissions_do_not_create_profile() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "permissions-empty", "from yaml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_metadata_at( + skill_dir, + r#" +permissions: {} +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + assert_eq!(outcome.skills[0].permission_profile, None); + assert_eq!(outcome.skills[0].permissions, None); + } + + #[tokio::test] + async fn loads_exact_skill_permissions_from_yaml() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "permissions-skill-exact", "from yaml"); + let skill_dir = skill_path.parent().expect("skill dir"); + fs::create_dir_all(skill_dir.join("data")).expect("create read path"); + fs::create_dir_all(skill_dir.join("output")).expect("create write path"); + + write_skill_metadata_at( + skill_dir, + r#" +permissions: + mode: exact + network: true + file_system: + read: + - "./data" + write: + - "./output" +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); #[cfg(target_os = "macos")] let macos_seatbelt_profile_extensions = Some(crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default()); @@ -1433,15 +1520,16 @@ permissions: } #[tokio::test] - async fn empty_skill_permissions_do_not_create_profile() { + async fn empty_exact_skill_permissions_compile_default_sandbox() { let codex_home = tempfile::tempdir().expect("tempdir"); - let skill_path = write_skill(&codex_home, "demo", "permissions-empty", "from yaml"); + let skill_path = write_skill(&codex_home, "demo", "permissions-empty-exact", "from yaml"); let skill_dir = skill_path.parent().expect("skill dir"); write_skill_metadata_at( skill_dir, r#" -permissions: {} +permissions: + mode: exact "#, ); @@ -1495,6 +1583,7 @@ permissions: {} skill_dir, r#" permissions: + mode: exact macos: preferences: "readwrite" automations: @@ -1545,6 +1634,7 @@ permissions: skill_dir, r#" permissions: + mode: exact macos: preferences: "readwrite" automations: diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 38d02ee0b6..f1165dcd76 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -1,10 +1,16 @@ +#[cfg(target_os = "macos")] +use std::collections::BTreeSet; use std::collections::HashSet; use std::path::Path; +#[cfg(target_os = "macos")] +use codex_protocol::models::MacOsAutomationPermission; #[cfg(target_os = "macos")] use codex_protocol::models::MacOsAutomationValue; use codex_protocol::models::MacOsPermissions; #[cfg(target_os = "macos")] +use codex_protocol::models::MacOsPreferencesPermission; +#[cfg(target_os = "macos")] use codex_protocol::models::MacOsPreferencesValue; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; @@ -23,11 +29,12 @@ pub(crate) fn compile_permission_profile( _skill_dir: &Path, permissions: Option, ) -> Option { + let permissions = normalize_permission_profile(permissions?); let PermissionProfile { network, file_system, macos, - } = permissions?; + } = permissions; let file_system = file_system.unwrap_or_default(); let fs_read = normalize_permission_paths( file_system.read.as_deref().unwrap_or_default(), @@ -78,6 +85,36 @@ pub(crate) fn compile_permission_profile( }) } +pub(crate) fn normalize_permission_profile(permissions: PermissionProfile) -> PermissionProfile { + let PermissionProfile { + network, + file_system, + macos, + } = permissions; + let file_system = file_system.and_then(|file_system| { + let read = file_system + .read + .map(|paths| normalize_permission_paths(&paths, "permissions.file_system.read")) + .filter(|paths| !paths.is_empty()); + let write = file_system + .write + .map(|paths| normalize_permission_paths(&paths, "permissions.file_system.write")) + .filter(|paths| !paths.is_empty()); + + if read.is_none() && write.is_none() { + None + } else { + Some(codex_protocol::models::FileSystemPermissions { read, write }) + } + }); + + PermissionProfile { + network, + file_system, + macos, + } +} + fn normalize_permission_paths(values: &[AbsolutePathBuf], field: &str) -> Vec { let mut paths = Vec::new(); let mut seen = HashSet::new(); @@ -128,6 +165,117 @@ fn build_macos_seatbelt_profile_extensions( Some(extensions) } +#[cfg(target_os = "macos")] +pub(crate) fn merge_macos_seatbelt_profile_extensions( + current: Option<&MacOsSeatbeltProfileExtensions>, + permissions: Option<&MacOsPermissions>, +) -> Option { + let Some(permissions) = permissions else { + return current.cloned(); + }; + + let defaults = current.cloned().unwrap_or_default(); + let merged = MacOsSeatbeltProfileExtensions { + macos_preferences: merge_macos_preferences_permission( + permissions.preferences.as_ref(), + defaults.macos_preferences, + ), + macos_automation: merge_macos_automation_permission( + permissions.automations.as_ref(), + defaults.macos_automation, + ), + macos_accessibility: defaults.macos_accessibility + || permissions.accessibility.unwrap_or(false), + macos_calendar: defaults.macos_calendar || permissions.calendar.unwrap_or(false), + }; + Some(merged) +} + +#[cfg(target_os = "macos")] +fn merge_macos_preferences_permission( + value: Option<&MacOsPreferencesValue>, + current: MacOsPreferencesPermission, +) -> MacOsPreferencesPermission { + let requested = match value { + Some(MacOsPreferencesValue::Bool(true)) => Some(MacOsPreferencesPermission::ReadOnly), + Some(MacOsPreferencesValue::Bool(false)) | None => None, + Some(MacOsPreferencesValue::Mode(mode)) => { + let mode = mode.trim(); + if mode.eq_ignore_ascii_case("readonly") || mode.eq_ignore_ascii_case("read-only") { + Some(MacOsPreferencesPermission::ReadOnly) + } else if mode.eq_ignore_ascii_case("readwrite") + || mode.eq_ignore_ascii_case("read-write") + { + Some(MacOsPreferencesPermission::ReadWrite) + } else { + warn!( + "ignoring permissions.macos.preferences: expected true/false, readonly, or readwrite" + ); + None + } + } + }; + + match (current, requested) { + (MacOsPreferencesPermission::ReadWrite, _) => MacOsPreferencesPermission::ReadWrite, + (MacOsPreferencesPermission::ReadOnly, Some(MacOsPreferencesPermission::ReadWrite)) => { + MacOsPreferencesPermission::ReadWrite + } + (MacOsPreferencesPermission::None, Some(permission)) => permission, + (existing, Some(_)) | (existing, None) => existing, + } +} + +#[cfg(target_os = "macos")] +fn merge_macos_automation_permission( + value: Option<&MacOsAutomationValue>, + current: MacOsAutomationPermission, +) -> MacOsAutomationPermission { + let requested = match value { + Some(MacOsAutomationValue::Bool(true)) => Some(MacOsAutomationPermission::All), + Some(MacOsAutomationValue::Bool(false)) | None => None, + Some(MacOsAutomationValue::BundleIds(bundle_ids)) => { + let bundle_ids = bundle_ids + .iter() + .map(|bundle_id| bundle_id.trim()) + .filter(|bundle_id| !bundle_id.is_empty()) + .map(ToOwned::to_owned) + .collect::>(); + if bundle_ids.is_empty() { + None + } else { + Some(MacOsAutomationPermission::BundleIds(bundle_ids)) + } + } + }; + + match (current, requested) { + (MacOsAutomationPermission::All, _) | (_, Some(MacOsAutomationPermission::All)) => { + MacOsAutomationPermission::All + } + (MacOsAutomationPermission::None, Some(permission)) => permission, + ( + MacOsAutomationPermission::BundleIds(existing), + Some(MacOsAutomationPermission::BundleIds(requested)), + ) => { + let merged = existing + .into_iter() + .chain(requested) + .map(|bundle_id| bundle_id.trim().to_string()) + .filter(|bundle_id| !bundle_id.is_empty()) + .collect::>() + .into_iter() + .collect::>(); + if merged.is_empty() { + MacOsAutomationPermission::None + } else { + MacOsAutomationPermission::BundleIds(merged) + } + } + (existing, Some(_)) | (existing, None) => existing, + } +} + #[cfg(target_os = "macos")] fn resolve_macos_preferences_permission( value: Option<&MacOsPreferencesValue>, @@ -191,6 +339,14 @@ fn build_macos_seatbelt_profile_extensions( None } +#[cfg(not(target_os = "macos"))] +pub(crate) fn merge_macos_seatbelt_profile_extensions( + current: Option<&MacOsSeatbeltProfileExtensions>, + _: Option<&MacOsPermissions>, +) -> Option { + current.cloned() +} + #[cfg(test)] mod tests { use super::compile_permission_profile; diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 5288fd8e82..05c59e915e 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -7,8 +7,10 @@ use crate::exec::SandboxType; use crate::exec::is_likely_sandbox_denied; use crate::features::Feature; use crate::sandboxing::SandboxPermissions; +use crate::sandboxing::sandbox_policy_with_additional_permissions; use crate::shell::ShellType; use crate::skills::SkillMetadata; +use crate::skills::permissions::merge_macos_seatbelt_profile_extensions; use crate::tools::runtimes::ExecveSessionApproval; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::SandboxAttempt; @@ -226,7 +228,11 @@ impl CoreShellActionProvider { } } - fn skill_escalation_execution(skill: &SkillMetadata) -> EscalationExecution { + fn skill_escalation_execution( + skill: &SkillMetadata, + turn_sandbox_policy: &SandboxPolicy, + turn_macos_seatbelt_profile_extensions: Option<&MacOsSeatbeltProfileExtensions>, + ) -> EscalationExecution { skill .permissions .as_ref() @@ -241,11 +247,21 @@ impl CoreShellActionProvider { )) }) .or_else(|| { - skill - .permission_profile - .clone() - .map(EscalationPermissions::PermissionProfile) - .map(EscalationExecution::Permissions) + skill.permission_profile.as_ref().map(|permission_profile| { + EscalationExecution::Permissions(EscalationPermissions::Permissions( + EscalatedPermissions { + sandbox_policy: sandbox_policy_with_additional_permissions( + turn_sandbox_policy, + permission_profile, + ), + macos_seatbelt_profile_extensions: + merge_macos_seatbelt_profile_extensions( + turn_macos_seatbelt_profile_extensions, + permission_profile.macos.as_ref(), + ), + }, + )) + }) }) .unwrap_or(EscalationExecution::TurnDefault) } @@ -265,6 +281,13 @@ impl CoreShellActionProvider { let turn = self.turn.clone(); let call_id = self.call_id.clone(); let approval_id = Some(Uuid::new_v4().to_string()); + let reason = match decision_source { + DecisionSource::SkillScript { skill } if skill.permissions.is_some() => Some( + "Use the skill's declared sandbox exactly; do not merge it with the current sandbox." + .to_string(), + ), + _ => None, + }; Ok(stopwatch .pause_for(async move { let available_decisions = vec![ @@ -288,7 +311,7 @@ impl CoreShellActionProvider { approval_id, command, workdir, - None, + reason, None, None, additional_permissions, @@ -470,7 +493,17 @@ impl EscalationPolicy for CoreShellActionProvider { let execution = approval .skill .as_ref() - .map(Self::skill_escalation_execution) + .map(|skill| { + Self::skill_escalation_execution( + skill, + &self.sandbox_policy, + self.turn + .config + .permissions + .macos_seatbelt_profile_extensions + .as_ref(), + ) + }) .unwrap_or(EscalationExecution::TurnDefault); return Ok(EscalationDecision::escalate(execution)); @@ -495,7 +528,15 @@ impl EscalationPolicy for CoreShellActionProvider { argv, workdir, skill.permission_profile.clone(), - Self::skill_escalation_execution(&skill), + Self::skill_escalation_execution( + &skill, + &self.sandbox_policy, + self.turn + .config + .permissions + .macos_seatbelt_profile_extensions + .as_ref(), + ), decision_source, ) .await; diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index 84e1d24085..198ff0a5e3 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -408,10 +408,10 @@ async fn shell_zsh_fork_skill_without_permissions_inherits_turn_sandbox() -> Res /// and writes to an unrelated folder fail, both before and after cached approval. #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> Result<()> { +async fn shell_zsh_fork_skill_session_approval_merges_skill_permissions() -> Result<()> { skip_if_no_network!(Ok(())); - let Some(runtime) = zsh_fork_runtime("zsh-fork explicit skill sandbox test")? else { + let Some(runtime) = zsh_fork_runtime("zsh-fork merged skill permissions test")? else { return Ok(()); }; @@ -519,12 +519,12 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R .to_string(); assert!( first_output.contains("allowed"), - "expected skill sandbox to permit writes to the approved folder, got output: {first_output:?}" + "expected merged skill permissions to permit writes to the approved folder, got output: {first_output:?}" ); assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); assert!( !blocked_path.exists(), - "first run should not write outside the explicit skill sandbox" + "first run should not write outside the merged skill and turn sandbox" ); assert!( !first_output.contains("blocked-created"), @@ -567,12 +567,12 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R .to_string(); assert!( second_output.contains("allowed"), - "expected cached skill approval to retain the explicit skill sandbox, got output: {second_output:?}" + "expected cached skill approval to retain the merged skill permissions, got output: {second_output:?}" ); assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); assert!( !blocked_path.exists(), - "cached session approval should not widen skill execution beyond the explicit skill sandbox" + "cached session approval should not widen skill execution beyond the merged skill and turn sandbox" ); assert!( !second_output.contains("blocked-created"), @@ -582,6 +582,185 @@ async fn shell_zsh_fork_skill_session_approval_enforces_skill_permissions() -> R Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_zsh_fork_skill_exact_mode_replaces_turn_sandbox() -> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("zsh-fork exact skill sandbox test")? else { + return Ok(()); + }; + + let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?; + let allowed_dir = outside_dir.path().join("allowed-output"); + let blocked_dir = outside_dir.path().join("blocked-output"); + fs::create_dir_all(&allowed_dir)?; + fs::create_dir_all(&blocked_dir)?; + + let allowed_path = allowed_dir.join("allowed.txt"); + let blocked_path = blocked_dir.join("blocked.txt"); + let allowed_path_quoted = shlex::try_join([allowed_path.to_string_lossy().as_ref()])?; + let blocked_path_quoted = shlex::try_join([blocked_path.to_string_lossy().as_ref()])?; + let script_contents = format!( + "#!/bin/sh\nprintf '%s' allowed > {allowed_path_quoted}\ncat {allowed_path_quoted}\nprintf '%s' forbidden > {blocked_path_quoted}\nif [ -f {blocked_path_quoted} ]; then echo blocked-created; fi\n" + ); + let allowed_dir_for_hook = allowed_dir.clone(); + let allowed_path_for_hook = allowed_path.clone(); + let blocked_path_for_hook = blocked_path.clone(); + let script_contents_for_hook = script_contents.clone(); + + let permissions_yaml = format!( + "permissions:\n mode: exact\n file_system:\n write:\n - \"{}\"\n", + allowed_dir.display() + ); + + let server = start_mock_server().await; + let test = build_zsh_fork_test( + &server, + runtime, + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + move |home| { + let _ = fs::remove_file(&allowed_path_for_hook); + let _ = fs::remove_file(&blocked_path_for_hook); + fs::create_dir_all(&allowed_dir_for_hook).unwrap(); + fs::create_dir_all(blocked_path_for_hook.parent().unwrap()).unwrap(); + write_skill_with_shell_script_contents( + home, + "mbolin-test-skill", + "sandboxed.sh", + &script_contents_for_hook, + ) + .unwrap(); + write_skill_metadata(home, "mbolin-test-skill", &permissions_yaml).unwrap(); + }, + ) + .await?; + + let (script_path_str, command) = skill_script_command(&test, "sandboxed.sh")?; + + let first_call_id = "zsh-fork-skill-exact-1"; + let first_arguments = shell_command_arguments(&command)?; + let first_mocks = mount_function_call_agent_response( + &server, + first_call_id, + &first_arguments, + "shell_command", + ) + .await; + + submit_turn_with_policies( + &test, + "use $mbolin-test-skill", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + ) + .await?; + + let maybe_approval = wait_for_exec_approval_request(&test).await; + let approval = match maybe_approval { + Some(approval) => approval, + None => panic!("expected exec approval request before completion"), + }; + assert_eq!(approval.call_id, first_call_id); + assert_eq!(approval.command, vec![script_path_str.clone()]); + assert_eq!( + approval.reason.as_deref(), + Some("Use the skill's declared sandbox exactly; do not merge it with the current sandbox.") + ); + assert_eq!( + approval.additional_permissions, + Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: None, + write: Some(vec![absolute_path(&allowed_dir)]), + }), + ..Default::default() + }) + ); + + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::ApprovedForSession, + }) + .await?; + + wait_for_turn_complete(&test).await; + + let first_output = first_mocks + .completion + .single_request() + .function_call_output(first_call_id)["output"] + .as_str() + .unwrap_or_default() + .to_string(); + assert!( + first_output.contains("allowed"), + "expected exact skill sandbox to permit writes to the approved folder, got output: {first_output:?}" + ); + assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); + assert!( + !blocked_path.exists(), + "exact mode should replace the full-access turn sandbox instead of merging it" + ); + assert!( + !first_output.contains("blocked-created"), + "blocked path should not have been created in exact mode: {first_output:?}" + ); + + let second_call_id = "zsh-fork-skill-exact-2"; + let second_arguments = shell_command_arguments(&command)?; + let second_mocks = mount_function_call_agent_response( + &server, + second_call_id, + &second_arguments, + "shell_command", + ) + .await; + + let _ = fs::remove_file(&allowed_path); + let _ = fs::remove_file(&blocked_path); + + submit_turn_with_policies( + &test, + "use $mbolin-test-skill", + AskForApproval::OnRequest, + SandboxPolicy::DangerFullAccess, + ) + .await?; + + let cached_approval = wait_for_exec_approval_request(&test).await; + assert!( + cached_approval.is_none(), + "expected second run to reuse the cached session approval" + ); + + let second_output = second_mocks + .completion + .single_request() + .function_call_output(second_call_id)["output"] + .as_str() + .unwrap_or_default() + .to_string(); + assert!( + second_output.contains("allowed"), + "expected cached exact skill approval to retain the exact skill sandbox, got output: {second_output:?}" + ); + assert_eq!(fs::read_to_string(&allowed_path)?, "allowed"); + assert!( + !blocked_path.exists(), + "cached session approval should continue to replace the turn sandbox in exact mode" + ); + assert!( + !second_output.contains("blocked-created"), + "blocked path should not have been created after cached exact approval: {second_output:?}" + ); + + Ok(()) +} + /// This stays narrow on purpose: the important check is that `WorkspaceWrite` /// continues to deny writes outside the workspace even under `zsh-fork`. #[cfg(unix)] diff --git a/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md b/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md index 90f9e8e863..193e829367 100644 --- a/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md +++ b/codex-rs/skills/src/assets/samples/skill-creator/references/openai_yaml.md @@ -23,6 +23,12 @@ dependencies: policy: allow_implicit_invocation: true + +permissions: + mode: "merge" + file_system: + write: + - "/absolute/path/for/skill-output" ``` ## Field descriptions and constraints @@ -47,3 +53,10 @@ Top-level constraints: - `policy.allow_implicit_invocation`: When false, the skill is not injected into the model context by default, but can still be invoked explicitly via `$skill`. Defaults to true. +- `permissions.mode`: Optional sandbox behavior for skill script approvals. + `merge` is the default and adds the declared permissions to the current turn sandbox. + `exact` uses only the sandbox compiled from the declared permissions instead of merging. +- `permissions.network`: Optional network access hint for the skill permission profile. +- `permissions.file_system.read`: Optional absolute paths the skill may read. +- `permissions.file_system.write`: Optional absolute paths the skill may write. +- `permissions.macos`: Optional macOS-specific permission hints for skill execution.