From 623d3f40719182003943258a6c837f3572e3d581 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 10 Feb 2026 23:10:35 +0000 Subject: [PATCH 1/7] feat: mem v2 - PR4 (#11369) # Memories migration plan (simplified global workflow) ## Target behavior - One shared memory root only: `~/.codex/memories/`. - No per-cwd memory buckets, no cwd hash handling. - Phase 1 candidate rules: - Not currently being processed unless the job lease is stale. - Rollout updated within the max-age window (currently 30 days). - Rollout idle for at least 12 hours (new constant). - Global cap: at most 64 stage-1 jobs in `running` state at any time (new invariant). - Stage-1 model output shape (new): - `rollout_slug` (accepted but ignored for now). - `rollout_summary`. - `raw_memory`. - Phase-1 artifacts written under the shared root: - `rollout_summaries/.md` for each rollout summary. - `raw_memories.md` containing appended/merged raw memory paragraphs. - Phase 2 runs one consolidation agent for the shared `memories/` directory. - Phase-2 lock is DB-backed with 1 hour lease and heartbeat/expiry. ## Current code map - Core startup pipeline: `core/src/memories/startup/mod.rs`. - Stage-1 request+parse: `core/src/memories/startup/extract.rs`, `core/src/memories/stage_one.rs`, templates in `core/templates/memories/`. - File materialization: `core/src/memories/storage.rs`, `core/src/memories/layout.rs`. - Scope routing (cwd/user): `core/src/memories/scope.rs`, `core/src/memories/startup/mod.rs`. - DB job lifecycle and scope queueing: `state/src/runtime/memory.rs`. ## PR plan ## PR 1: Correct phase-1 selection invariants (no behavior-breaking layout changes yet) - Add `PHASE_ONE_MIN_ROLLOUT_IDLE_HOURS: i64 = 12` in `core/src/memories/mod.rs`. - Thread this into `state::claim_stage1_jobs_for_startup(...)`. - Enforce idle-time filter in DB selection logic (not only in-memory filtering after `scan_limit`) so eligible threads are not starved by very recent threads. - Enforce global running cap of 64 at claim time in DB logic: - Count fresh `memory_stage1` running jobs. - Only allow new claims while count < cap. - Keep stale-lease takeover behavior intact. - Add/adjust tests in `state/src/runtime.rs`: - Idle filter inclusion/exclusion around 12h boundary. - Global running-cap guarantee. - Existing stale/fresh ownership behavior still passes. Acceptance criteria: - Startup never creates more than 64 fresh `memory_stage1` running jobs. - Threads updated <12h ago are skipped. - Threads older than 30d are skipped. ## PR 2: Stage-1 output contract + storage artifacts (forward-compatible) - Update parser/types to accept the new structured output while keeping backward compatibility: - Add `rollout_slug` (optional for now). - Add `rollout_summary`. - Keep alias support for legacy `summary` and `rawMemory` until prompt swap completes. - Update stage-1 schema generator in `core/src/memories/stage_one.rs` to include the new keys. - Update prompt templates: - `core/templates/memories/stage_one_system.md`. - `core/templates/memories/stage_one_input.md`. - Replace storage model in `core/src/memories/storage.rs`: - Introduce `rollout_summaries/` directory writer (`.md` files). - Introduce `raw_memories.md` aggregator writer from DB rows. - Keep deterministic rebuild behavior from DB outputs so files can always be regenerated. - Update consolidation prompt template to reference `rollout_summaries/` + `raw_memories.md` inputs. Acceptance criteria: - Stage-1 accepts both old and new output keys during migration. - Phase-1 artifacts are generated in new format from DB state. - No dependence on per-thread files in `raw_memories/`. ## PR 3: Remove per-cwd memories and move to one global memory root - Simplify layout in `core/src/memories/layout.rs`: - Single root: `codex_home/memories`. - Remove cwd-hash bucket helpers and normalization logic used only for memory pathing. - Remove scope branching from startup phase-2 dispatch path: - No cwd/user mapping in `core/src/memories/startup/mod.rs`. - One target root for consolidation. - In `state/src/runtime/memory.rs`, stop enqueueing/handling cwd consolidation scope. - Keep one logical consolidation scope/job key (global/user) to avoid a risky schema rewrite in same PR. - Add one-time migration helper (core side) to preserve current shared memory output: - If `~/.codex/memories/user/memory` exists and new root is empty, move/copy contents into `~/.codex/memories`. - Leave old hashed cwd buckets untouched for now (safe/no-destructive migration). Acceptance criteria: - New runs only read/write `~/.codex/memories`. - No new cwd-scoped consolidation jobs are enqueued. - Existing user-shared memory content is preserved. ## PR 4: Phase-2 global lock simplification and cleanup - Replace multi-scope dispatch with a single global consolidation claim path: - Either reuse jobs table with one fixed key, or add a tiny dedicated lock helper; keep 1h lease. - Ensure at most one consolidation agent can run at once. - Keep heartbeat + stale lock recovery semantics in `core/src/memories/startup/watch.rs`. - Remove dead scope code and legacy constants no longer used. - Update tests: - One-agent-at-a-time behavior. - Lock expiry allows takeover after stale lease. Acceptance criteria: - Exactly one phase-2 consolidation agent can be active cluster-wide (per local DB). - Stale lock recovers automatically. ## PR 5: Final cleanup and docs - Remove legacy artifacts and references: - `raw_memories/` and `memory_summary.md` assumptions from prompts/comments/tests. - Scope constants for cwd memory pathing in core/state if fully unused. - Update docs under `docs/` for memory workflow and directory layout. - Add a brief operator note for rollout: compatibility window for old stage-1 JSON keys and when to remove aliases. Acceptance criteria: - Code and docs reflect only the simplified global workflow. - No stale references to per-cwd memory buckets. ## Notes on sequencing - PR 1 is safest first because it improves correctness without changing external artifact layout. - PR 2 keeps parser compatibility so prompt deployment can happen independently. - PR 3 and PR 4 split filesystem/scope simplification from locking simplification to reduce blast radius. - PR 5 is intentionally cleanup-only. --- codex-rs/core/src/memories/layout.rs | 5 +- codex-rs/core/src/memories/mod.rs | 9 +- codex-rs/core/src/memories/scope.rs | 2 - .../core/src/memories/startup/dispatch.rs | 328 ++++++++++++------ codex-rs/core/src/memories/startup/mod.rs | 160 +-------- codex-rs/core/src/memories/startup/watch.rs | 53 +-- codex-rs/core/src/memories/storage.rs | 6 +- codex-rs/state/src/lib.rs | 1 - codex-rs/state/src/runtime.rs | 210 +++++------ codex-rs/state/src/runtime/memory.rs | 176 ++-------- 10 files changed, 384 insertions(+), 566 deletions(-) delete mode 100644 codex-rs/core/src/memories/scope.rs diff --git a/codex-rs/core/src/memories/layout.rs b/codex-rs/core/src/memories/layout.rs index 94fd73890c..07a8bd4ec0 100644 --- a/codex-rs/core/src/memories/layout.rs +++ b/codex-rs/core/src/memories/layout.rs @@ -1,13 +1,12 @@ use std::path::Path; use std::path::PathBuf; -use super::scope::MEMORY_SCOPE_KEY_USER; - pub(super) const ROLLOUT_SUMMARIES_SUBDIR: &str = "rollout_summaries"; pub(super) const RAW_MEMORIES_FILENAME: &str = "raw_memories.md"; pub(super) const MEMORY_REGISTRY_FILENAME: &str = "MEMORY.md"; pub(super) const LEGACY_CONSOLIDATED_FILENAME: &str = "consolidated.md"; pub(super) const SKILLS_SUBDIR: &str = "skills"; +const LEGACY_USER_SUBDIR: &str = "user"; const LEGACY_MEMORY_SUBDIR: &str = "memory"; /// Returns the shared on-disk memory root directory. @@ -46,7 +45,7 @@ pub(super) async fn ensure_layout(root: &Path) -> std::io::Result<()> { fn legacy_user_memory_root(codex_home: &Path) -> PathBuf { codex_home .join("memories") - .join(MEMORY_SCOPE_KEY_USER) + .join(LEGACY_USER_SUBDIR) .join(LEGACY_MEMORY_SUBDIR) } diff --git a/codex-rs/core/src/memories/mod.rs b/codex-rs/core/src/memories/mod.rs index 7867f90641..cfda3f47eb 100644 --- a/codex-rs/core/src/memories/mod.rs +++ b/codex-rs/core/src/memories/mod.rs @@ -2,12 +2,11 @@ //! //! The startup memory pipeline is split into two phases: //! - Phase 1: select rollouts, extract stage-1 raw memories, persist stage-1 outputs, and enqueue consolidation. -//! - Phase 2: claim scopes, materialize consolidation inputs, and dispatch consolidation agents. +//! - Phase 2: claim a global consolidation lock, materialize consolidation inputs, and dispatch one consolidation agent. mod layout; mod prompts; mod rollout; -mod scope; mod stage_one; mod startup; mod storage; @@ -23,10 +22,8 @@ const MEMORY_CONSOLIDATION_SUBAGENT_LABEL: &str = "memory_consolidation"; const MAX_ROLLOUTS_PER_STARTUP: usize = 64; /// Concurrency cap for startup memory extraction and consolidation scheduling. const PHASE_ONE_CONCURRENCY_LIMIT: usize = MAX_ROLLOUTS_PER_STARTUP; -/// Concurrency cap for phase-2 consolidation dispatch. -const PHASE_TWO_CONCURRENCY_LIMIT: usize = MAX_ROLLOUTS_PER_STARTUP; -/// Maximum number of recent raw memories retained per scope. -const MAX_RAW_MEMORIES_PER_SCOPE: usize = 64; +/// Maximum number of recent raw memories retained for global consolidation. +const MAX_RAW_MEMORIES_FOR_GLOBAL: usize = 64; /// Maximum rollout age considered for phase-1 extraction. const PHASE_ONE_MAX_ROLLOUT_AGE_DAYS: i64 = 30; /// Minimum rollout idle time required before phase-1 extraction. diff --git a/codex-rs/core/src/memories/scope.rs b/codex-rs/core/src/memories/scope.rs deleted file mode 100644 index 39e413a34f..0000000000 --- a/codex-rs/core/src/memories/scope.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub(super) const MEMORY_SCOPE_KIND_USER: &str = "user"; -pub(super) const MEMORY_SCOPE_KEY_USER: &str = "user"; diff --git a/codex-rs/core/src/memories/startup/dispatch.rs b/codex-rs/core/src/memories/startup/dispatch.rs index 0c8a4d3276..c18b9b77d8 100644 --- a/codex-rs/core/src/memories/startup/dispatch.rs +++ b/codex-rs/core/src/memories/startup/dispatch.rs @@ -1,5 +1,6 @@ use crate::codex::Session; use crate::config::Config; +use crate::memories::layout::memory_root; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_protocol::user_input::UserInput; @@ -8,7 +9,7 @@ use tracing::debug; use tracing::info; use tracing::warn; -use super::super::MAX_RAW_MEMORIES_PER_SCOPE; +use super::super::MAX_RAW_MEMORIES_FOR_GLOBAL; use super::super::MEMORY_CONSOLIDATION_SUBAGENT_LABEL; use super::super::PHASE_TWO_JOB_LEASE_SECONDS; use super::super::PHASE_TWO_JOB_RETRY_DELAY_SECONDS; @@ -16,38 +17,25 @@ use super::super::prompts::build_consolidation_prompt; use super::super::storage::rebuild_raw_memories_file_from_memories; use super::super::storage::sync_rollout_summaries_from_memories; use super::super::storage::wipe_consolidation_outputs; -use super::MemoryScopeTarget; use super::watch::spawn_phase2_completion_task; -pub(super) async fn run_memory_consolidation_for_scope( - session: Arc, +pub(super) async fn run_global_memory_consolidation( + session: &Arc, config: Arc, - scope: MemoryScopeTarget, -) { +) -> bool { let Some(state_db) = session.services.state_db.as_deref() else { - warn!( - "state db unavailable for scope {}:{}; skipping consolidation", - scope.scope_kind, scope.scope_key - ); - return; + warn!("state db unavailable; skipping global memory consolidation"); + return false; }; let claim = match state_db - .try_claim_phase2_job( - scope.scope_kind, - &scope.scope_key, - session.conversation_id, - PHASE_TWO_JOB_LEASE_SECONDS, - ) + .try_claim_global_phase2_job(session.conversation_id, PHASE_TWO_JOB_LEASE_SECONDS) .await { Ok(claim) => claim, Err(err) => { - warn!( - "state db try_claim_phase2_job failed for scope {}:{}: {err}", - scope.scope_kind, scope.scope_key - ); - return; + warn!("state db try_claim_global_phase2_job failed during memories startup: {err}"); + return false; } }; let (ownership_token, claimed_watermark) = match claim { @@ -56,131 +44,90 @@ pub(super) async fn run_memory_consolidation_for_scope( input_watermark, } => (ownership_token, input_watermark), codex_state::Phase2JobClaimOutcome::SkippedNotDirty => { - debug!( - "memory phase-2 scope not pending (or already up to date); skipping consolidation: {}:{}", - scope.scope_kind, scope.scope_key - ); - return; + debug!("memory phase-2 global lock is up-to-date; skipping consolidation"); + return false; } codex_state::Phase2JobClaimOutcome::SkippedRunning => { - debug!( - "memory phase-2 job already running for scope {}:{}; skipping", - scope.scope_kind, scope.scope_key - ); - return; + debug!("memory phase-2 global consolidation already running; skipping"); + return false; } }; let latest_memories = match state_db - .list_stage1_outputs_for_scope( - scope.scope_kind, - &scope.scope_key, - MAX_RAW_MEMORIES_PER_SCOPE, - ) + .list_stage1_outputs_for_global(MAX_RAW_MEMORIES_FOR_GLOBAL) .await { Ok(memories) => memories, Err(err) => { - warn!( - "state db list_stage1_outputs_for_scope failed during consolidation for scope {}:{}: {err}", - scope.scope_kind, scope.scope_key - ); + warn!("state db list_stage1_outputs_for_global failed during consolidation: {err}"); let _ = state_db - .mark_phase2_job_failed( - scope.scope_kind, - &scope.scope_key, + .mark_global_phase2_job_failed( &ownership_token, - "failed to read scope stage-1 outputs before consolidation", + "failed to read stage-1 outputs before global consolidation", PHASE_TWO_JOB_RETRY_DELAY_SECONDS, ) .await; - return; + return false; } }; if latest_memories.is_empty() { - debug!( - "memory phase-2 scope has no stage-1 outputs; skipping consolidation: {}:{}", - scope.scope_kind, scope.scope_key - ); + debug!("memory phase-2 has no stage-1 outputs; skipping global consolidation"); let _ = state_db - .mark_phase2_job_succeeded( - scope.scope_kind, - &scope.scope_key, - &ownership_token, - claimed_watermark, - ) + .mark_global_phase2_job_succeeded(&ownership_token, claimed_watermark) .await; - return; + return false; }; + let root = memory_root(&config.codex_home); let materialized_watermark = latest_memories .iter() .map(|memory| memory.source_updated_at.timestamp()) .max() .unwrap_or(claimed_watermark); - if let Err(err) = - sync_rollout_summaries_from_memories(&scope.memory_root, &latest_memories).await - { - warn!( - "failed syncing phase-1 rollout summaries for scope {}:{}: {err}", - scope.scope_kind, scope.scope_key - ); + if let Err(err) = sync_rollout_summaries_from_memories(&root, &latest_memories).await { + warn!("failed syncing phase-1 rollout summaries for global consolidation: {err}"); let _ = state_db - .mark_phase2_job_failed( - scope.scope_kind, - &scope.scope_key, + .mark_global_phase2_job_failed( &ownership_token, "failed syncing phase-1 rollout summaries", PHASE_TWO_JOB_RETRY_DELAY_SECONDS, ) .await; - return; + return false; } - if let Err(err) = - rebuild_raw_memories_file_from_memories(&scope.memory_root, &latest_memories).await - { - warn!( - "failed rebuilding raw memories aggregate for scope {}:{}: {err}", - scope.scope_kind, scope.scope_key - ); + if let Err(err) = rebuild_raw_memories_file_from_memories(&root, &latest_memories).await { + warn!("failed rebuilding raw memories aggregate for global consolidation: {err}"); let _ = state_db - .mark_phase2_job_failed( - scope.scope_kind, - &scope.scope_key, + .mark_global_phase2_job_failed( &ownership_token, "failed rebuilding raw memories aggregate", PHASE_TWO_JOB_RETRY_DELAY_SECONDS, ) .await; - return; + return false; } - if let Err(err) = wipe_consolidation_outputs(&scope.memory_root).await { - warn!( - "failed to wipe previous consolidation outputs for scope {}:{}: {err}", - scope.scope_kind, scope.scope_key - ); + if let Err(err) = wipe_consolidation_outputs(&root).await { + warn!("failed to wipe previous global consolidation outputs: {err}"); let _ = state_db - .mark_phase2_job_failed( - scope.scope_kind, - &scope.scope_key, + .mark_global_phase2_job_failed( &ownership_token, "failed to wipe previous consolidation outputs", PHASE_TWO_JOB_RETRY_DELAY_SECONDS, ) .await; - return; + return false; } - let prompt = build_consolidation_prompt(&scope.memory_root); + let prompt = build_consolidation_prompt(&root); let input = vec![UserInput::Text { text: prompt, text_elements: vec![], }]; let mut consolidation_config = config.as_ref().clone(); - consolidation_config.cwd = scope.memory_root.clone(); + consolidation_config.cwd = root.clone(); let source = SessionSource::SubAgent(SubAgentSource::Other( MEMORY_CONSOLIDATION_SUBAGENT_LABEL.to_string(), )); @@ -193,31 +140,212 @@ pub(super) async fn run_memory_consolidation_for_scope( { Ok(consolidation_agent_id) => { info!( - "memory phase-2 consolidation agent started: scope={} scope_key={} agent_id={}", - scope.scope_kind, scope.scope_key, consolidation_agent_id + "memory phase-2 global consolidation agent started: agent_id={consolidation_agent_id}" ); spawn_phase2_completion_task( session.as_ref(), - scope, ownership_token, materialized_watermark, consolidation_agent_id, ); + true } Err(err) => { - warn!( - "failed to spawn memory consolidation agent for scope {}:{}: {err}", - scope.scope_kind, scope.scope_key - ); + warn!("failed to spawn global memory consolidation agent: {err}"); let _ = state_db - .mark_phase2_job_failed( - scope.scope_kind, - &scope.scope_key, + .mark_global_phase2_job_failed( &ownership_token, "failed to spawn consolidation agent", PHASE_TWO_JOB_RETRY_DELAY_SECONDS, ) .await; + false } } } + +#[cfg(test)] +mod tests { + use super::run_global_memory_consolidation; + use crate::CodexAuth; + use crate::ThreadManager; + use crate::codex::Session; + use crate::codex::make_session_and_context; + use crate::config::Config; + use crate::config::test_config; + use chrono::Utc; + use codex_protocol::ThreadId; + use codex_protocol::protocol::Op; + use codex_protocol::protocol::SessionSource; + use codex_state::Phase2JobClaimOutcome; + use codex_state::ThreadMetadataBuilder; + use pretty_assertions::assert_eq; + use std::sync::Arc; + use tempfile::TempDir; + + struct DispatchHarness { + _codex_home: TempDir, + config: Arc, + session: Arc, + manager: ThreadManager, + state_db: Arc, + } + + impl DispatchHarness { + async fn new() -> Self { + let codex_home = tempfile::tempdir().expect("create temp codex home"); + let mut config = test_config(); + config.codex_home = codex_home.path().to_path_buf(); + config.cwd = config.codex_home.clone(); + let config = Arc::new(config); + + let state_db = codex_state::StateRuntime::init( + config.codex_home.clone(), + config.model_provider_id.clone(), + None, + ) + .await + .expect("initialize state db"); + + let manager = ThreadManager::with_models_provider_and_home( + CodexAuth::from_api_key("dummy"), + config.model_provider.clone(), + config.codex_home.clone(), + ); + let (mut session, _turn_context) = make_session_and_context().await; + session.services.state_db = Some(Arc::clone(&state_db)); + session.services.agent_control = manager.agent_control(); + + Self { + _codex_home: codex_home, + config, + session: Arc::new(session), + manager, + state_db, + } + } + + async fn seed_stage1_output(&self, source_updated_at: i64) { + let thread_id = ThreadId::new(); + let mut metadata_builder = ThreadMetadataBuilder::new( + thread_id, + self.config + .codex_home + .join(format!("rollout-{thread_id}.jsonl")), + Utc::now(), + SessionSource::Cli, + ); + metadata_builder.cwd = self.config.cwd.clone(); + metadata_builder.model_provider = Some(self.config.model_provider_id.clone()); + let metadata = metadata_builder.build(&self.config.model_provider_id); + + self.state_db + .upsert_thread(&metadata) + .await + .expect("upsert thread metadata"); + + let claim = self + .state_db + .try_claim_stage1_job( + thread_id, + self.session.conversation_id, + source_updated_at, + 3_600, + 64, + ) + .await + .expect("claim stage-1 job"); + let ownership_token = match claim { + codex_state::Stage1JobClaimOutcome::Claimed { ownership_token } => ownership_token, + other => panic!("unexpected stage-1 claim outcome: {other:?}"), + }; + assert!( + self.state_db + .mark_stage1_job_succeeded( + thread_id, + &ownership_token, + source_updated_at, + "raw memory", + "rollout summary", + ) + .await + .expect("mark stage-1 success"), + "stage-1 success should enqueue global consolidation" + ); + } + + async fn shutdown_threads(&self) { + self.manager + .remove_and_close_all_threads() + .await + .expect("shutdown spawned threads"); + } + } + + #[tokio::test] + async fn dispatch_reclaims_stale_global_lock_and_starts_consolidation() { + let harness = DispatchHarness::new().await; + harness.seed_stage1_output(100).await; + + let stale_claim = harness + .state_db + .try_claim_global_phase2_job(ThreadId::new(), 0) + .await + .expect("claim stale global lock"); + assert!( + matches!(stale_claim, Phase2JobClaimOutcome::Claimed { .. }), + "stale lock precondition should be claimed" + ); + + let scheduled = + run_global_memory_consolidation(&harness.session, Arc::clone(&harness.config)).await; + assert!( + scheduled, + "dispatch should reclaim stale lock and spawn one agent" + ); + + let running_claim = harness + .state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim while running"); + assert_eq!(running_claim, Phase2JobClaimOutcome::SkippedRunning); + + let user_input_ops = harness + .manager + .captured_ops() + .into_iter() + .filter(|(_, op)| matches!(op, Op::UserInput { .. })) + .count(); + assert_eq!(user_input_ops, 1); + + harness.shutdown_threads().await; + } + + #[tokio::test] + async fn dispatch_schedules_only_one_agent_while_lock_is_running() { + let harness = DispatchHarness::new().await; + harness.seed_stage1_output(200).await; + + let first_run = + run_global_memory_consolidation(&harness.session, Arc::clone(&harness.config)).await; + let second_run = + run_global_memory_consolidation(&harness.session, Arc::clone(&harness.config)).await; + + assert!(first_run, "first dispatch should schedule consolidation"); + assert!( + !second_run, + "second dispatch should skip while the global lock is running" + ); + + let user_input_ops = harness + .manager + .captured_ops() + .into_iter() + .filter(|(_, op)| matches!(op, Op::UserInput { .. })) + .count(); + assert_eq!(user_input_ops, 1); + + harness.shutdown_threads().await; + } +} diff --git a/codex-rs/core/src/memories/startup/mod.rs b/codex-rs/core/src/memories/startup/mod.rs index f2ed60ea15..985d4d2758 100644 --- a/codex-rs/core/src/memories/startup/mod.rs +++ b/codex-rs/core/src/memories/startup/mod.rs @@ -7,10 +7,7 @@ use crate::codex::TurnContext; use crate::config::Config; use crate::error::Result as CodexResult; use crate::features::Feature; -use crate::memories::layout::memory_root; use crate::memories::layout::migrate_legacy_user_memory_root_if_needed; -use crate::memories::scope::MEMORY_SCOPE_KEY_USER; -use crate::memories::scope::MEMORY_SCOPE_KIND_USER; use crate::rollout::INTERACTIVE_SESSION_SOURCES; use codex_otel::OtelManager; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; @@ -19,7 +16,6 @@ use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::protocol::SessionSource; use futures::StreamExt; use serde_json::Value; -use std::path::PathBuf; use std::sync::Arc; use tracing::info; use tracing::warn; @@ -47,52 +43,6 @@ impl StageOneRequestContext { } } -/// Canonical memory scope metadata used by both startup phases. -#[derive(Clone, Debug, PartialEq, Eq)] -pub(super) struct MemoryScopeTarget { - /// Scope family used for DB ownership and dirty-state tracking. - pub(super) scope_kind: &'static str, - /// Scope identifier used for DB keys. - pub(super) scope_key: String, - /// On-disk root where phase-1 artifacts and phase-2 outputs live. - pub(super) memory_root: PathBuf, -} - -/// Converts a pending scope consolidation row into a concrete filesystem target for phase 2. -/// -/// Unsupported scope kinds or malformed keys are ignored. -pub(super) fn memory_scope_target_for_pending_scope( - config: &Config, - pending_scope: codex_state::PendingScopeConsolidation, -) -> Option { - let scope_kind = pending_scope.scope_kind; - let scope_key = pending_scope.scope_key; - - match scope_kind.as_str() { - MEMORY_SCOPE_KIND_USER => { - if scope_key != MEMORY_SCOPE_KEY_USER { - warn!( - "skipping unsupported user memory scope key for phase-2: {}:{}", - scope_kind, scope_key - ); - return None; - } - Some(MemoryScopeTarget { - scope_kind: MEMORY_SCOPE_KIND_USER, - scope_key, - memory_root: memory_root(&config.codex_home), - }) - } - _ => { - warn!( - "skipping unsupported memory scope for phase-2 consolidation: {}:{}", - scope_kind, scope_key - ); - None - } - } -} - /// Starts the asynchronous startup memory pipeline for an eligible root session. /// /// The pipeline is skipped for ephemeral sessions, disabled feature flags, and @@ -125,7 +75,7 @@ pub(crate) fn start_memories_startup_task( /// Phase 1 selects rollout candidates, performs stage-1 extraction requests in /// parallel, persists stage-1 outputs, and enqueues consolidation work. /// -/// Phase 2 claims pending scopes and spawns consolidation agents. +/// Phase 2 claims a global consolidation lock and spawns one consolidation agent. pub(super) async fn run_memories_startup_pipeline( session: &Arc, config: Arc, @@ -236,115 +186,15 @@ pub(super) async fn run_memories_startup_pipeline( claimed_count, succeeded_count ); - let consolidation_scope_count = run_consolidation_dispatch(session, config).await; + let consolidation_job_count = run_consolidation_dispatch(session, config).await; info!( - "memory consolidation dispatch complete: {} scope(s) scheduled", - consolidation_scope_count + "memory consolidation dispatch complete: {} job(s) scheduled", + consolidation_job_count ); Ok(()) } async fn run_consolidation_dispatch(session: &Arc, config: Arc) -> usize { - let scopes = list_consolidation_scopes( - session.as_ref(), - config.as_ref(), - super::MAX_ROLLOUTS_PER_STARTUP, - ) - .await; - let consolidation_scope_count = scopes.len(); - - futures::stream::iter(scopes.into_iter()) - .map(|scope| { - let session = Arc::clone(session); - let config = Arc::clone(&config); - async move { - dispatch::run_memory_consolidation_for_scope(session, config, scope).await; - } - }) - .buffer_unordered(super::PHASE_TWO_CONCURRENCY_LIMIT) - .collect::>() - .await; - - consolidation_scope_count -} - -async fn list_consolidation_scopes( - session: &Session, - config: &Config, - limit: usize, -) -> Vec { - if limit == 0 { - return Vec::new(); - } - - let Some(state_db) = session.services.state_db.as_deref() else { - return Vec::new(); - }; - - let pending_scopes = match state_db.list_pending_scope_consolidations(limit).await { - Ok(scopes) => scopes, - Err(_) => return Vec::new(), - }; - - pending_scopes - .into_iter() - .filter_map(|scope| memory_scope_target_for_pending_scope(config, scope)) - .collect() -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::config::test_config; - - /// Verifies that phase-2 pending scope rows are translated only for supported scopes. - #[test] - fn pending_scope_mapping_accepts_supported_scopes_only() { - let mut config = test_config(); - config.codex_home = "/tmp/memory-startup-test-home".into(); - - let user_target = memory_scope_target_for_pending_scope( - &config, - codex_state::PendingScopeConsolidation { - scope_kind: MEMORY_SCOPE_KIND_USER.to_string(), - scope_key: MEMORY_SCOPE_KEY_USER.to_string(), - }, - ) - .expect("valid user scope should map"); - assert_eq!(user_target.scope_kind, MEMORY_SCOPE_KIND_USER); - - assert!( - memory_scope_target_for_pending_scope( - &config, - codex_state::PendingScopeConsolidation { - scope_kind: MEMORY_SCOPE_KIND_USER.to_string(), - scope_key: "unexpected-user-key".to_string(), - }, - ) - .is_none() - ); - - assert!( - memory_scope_target_for_pending_scope( - &config, - codex_state::PendingScopeConsolidation { - scope_kind: "cwd".to_string(), - scope_key: "/tmp/project-a".to_string(), - }, - ) - .is_none() - ); - - assert!( - memory_scope_target_for_pending_scope( - &config, - codex_state::PendingScopeConsolidation { - scope_kind: "unknown".to_string(), - scope_key: "scope".to_string(), - }, - ) - .is_none() - ); - } + usize::from(dispatch::run_global_memory_consolidation(session, config).await) } diff --git a/codex-rs/core/src/memories/startup/watch.rs b/codex-rs/core/src/memories/startup/watch.rs index ea4341e11b..8450bab752 100644 --- a/codex-rs/core/src/memories/startup/watch.rs +++ b/codex-rs/core/src/memories/startup/watch.rs @@ -10,11 +10,9 @@ use tracing::warn; use super::super::PHASE_TWO_JOB_HEARTBEAT_SECONDS; use super::super::PHASE_TWO_JOB_LEASE_SECONDS; use super::super::PHASE_TWO_JOB_RETRY_DELAY_SECONDS; -use super::MemoryScopeTarget; pub(super) fn spawn_phase2_completion_task( session: &Session, - scope: MemoryScopeTarget, ownership_token: String, completion_watermark: i64, consolidation_agent_id: ThreadId, @@ -31,13 +29,10 @@ pub(super) fn spawn_phase2_completion_task( Ok(status_rx) => status_rx, Err(err) => { warn!( - "failed to subscribe to memory consolidation agent {} for scope {}:{}: {err}", - consolidation_agent_id, scope.scope_kind, scope.scope_key + "failed to subscribe to global memory consolidation agent {consolidation_agent_id}: {err}" ); let _ = state_db - .mark_phase2_job_failed( - scope.scope_kind, - &scope.scope_key, + .mark_global_phase2_job_failed( &ownership_token, "failed to subscribe to consolidation agent status", PHASE_TWO_JOB_RETRY_DELAY_SECONDS, @@ -61,33 +56,26 @@ pub(super) fn spawn_phase2_completion_task( changed = status_rx.changed() => { if changed.is_err() { warn!( - "lost status updates for memory consolidation agent {} in scope {}:{}", - consolidation_agent_id, scope.scope_kind, scope.scope_key + "lost status updates for global memory consolidation agent {consolidation_agent_id}" ); break status; } } _ = heartbeat_interval.tick() => { match state_db - .heartbeat_phase2_job( - scope.scope_kind, - &scope.scope_key, - &ownership_token, - PHASE_TWO_JOB_LEASE_SECONDS, - ) + .heartbeat_global_phase2_job(&ownership_token, PHASE_TWO_JOB_LEASE_SECONDS) .await { Ok(true) => {} Ok(false) => { debug!( - "memory phase-2 heartbeat lost ownership for scope {}:{}; skipping finalization", - scope.scope_kind, scope.scope_key + "memory phase-2 heartbeat lost global ownership; skipping finalization" ); return; } Err(err) => { warn!( - "state db heartbeat_phase2_job failed during memories startup: {err}" + "state db heartbeat_global_phase2_job failed during memories startup: {err}" ); return; } @@ -98,39 +86,30 @@ pub(super) fn spawn_phase2_completion_task( if is_phase2_success(&final_status) { match state_db - .mark_phase2_job_succeeded( - scope.scope_kind, - &scope.scope_key, - &ownership_token, - completion_watermark, - ) + .mark_global_phase2_job_succeeded(&ownership_token, completion_watermark) .await { Ok(true) => {} Ok(false) => { debug!( - "memory phase-2 success finalization skipped after ownership changed: scope={} scope_key={}", - scope.scope_kind, scope.scope_key + "memory phase-2 success finalization skipped after global ownership changed" ); } Err(err) => { warn!( - "state db mark_phase2_job_succeeded failed during memories startup: {err}" + "state db mark_global_phase2_job_succeeded failed during memories startup: {err}" ); } } info!( - "memory phase-2 consolidation agent finished: scope={} scope_key={} agent_id={} final_status={final_status:?}", - scope.scope_kind, scope.scope_key, consolidation_agent_id + "memory phase-2 global consolidation agent finished: agent_id={consolidation_agent_id} final_status={final_status:?}" ); return; } let failure_reason = phase2_failure_reason(&final_status); match state_db - .mark_phase2_job_failed( - scope.scope_kind, - &scope.scope_key, + .mark_global_phase2_job_failed( &ownership_token, &failure_reason, PHASE_TWO_JOB_RETRY_DELAY_SECONDS, @@ -140,17 +119,17 @@ pub(super) fn spawn_phase2_completion_task( Ok(true) => {} Ok(false) => { debug!( - "memory phase-2 failure finalization skipped after ownership changed: scope={} scope_key={}", - scope.scope_kind, scope.scope_key + "memory phase-2 failure finalization skipped after global ownership changed" ); } Err(err) => { - warn!("state db mark_phase2_job_failed failed during memories startup: {err}"); + warn!( + "state db mark_global_phase2_job_failed failed during memories startup: {err}" + ); } } warn!( - "memory phase-2 consolidation agent finished with non-success status: scope={} scope_key={} agent_id={} final_status={final_status:?}", - scope.scope_kind, scope.scope_key, consolidation_agent_id + "memory phase-2 global consolidation agent finished with non-success status: agent_id={consolidation_agent_id} final_status={final_status:?}" ); }); } diff --git a/codex-rs/core/src/memories/storage.rs b/codex-rs/core/src/memories/storage.rs index 9f78a98d6a..049130a5cf 100644 --- a/codex-rs/core/src/memories/storage.rs +++ b/codex-rs/core/src/memories/storage.rs @@ -4,7 +4,7 @@ use std::fmt::Write as _; use std::path::Path; use tracing::warn; -use super::MAX_RAW_MEMORIES_PER_SCOPE; +use super::MAX_RAW_MEMORIES_FOR_GLOBAL; use super::text::compact_whitespace; use crate::memories::layout::LEGACY_CONSOLIDATED_FILENAME; use crate::memories::layout::MEMORY_REGISTRY_FILENAME; @@ -31,7 +31,7 @@ pub(super) async fn sync_rollout_summaries_from_memories( let retained = memories .iter() - .take(MAX_RAW_MEMORIES_PER_SCOPE) + .take(MAX_RAW_MEMORIES_FOR_GLOBAL) .collect::>(); let keep = retained .iter() @@ -77,7 +77,7 @@ pub(super) async fn wipe_consolidation_outputs(root: &Path) -> std::io::Result<( async fn rebuild_raw_memories_file(root: &Path, memories: &[Stage1Output]) -> std::io::Result<()> { let retained = memories .iter() - .take(MAX_RAW_MEMORIES_PER_SCOPE) + .take(MAX_RAW_MEMORIES_FOR_GLOBAL) .collect::>(); let mut body = String::from("# Raw Memories\n\n"); diff --git a/codex-rs/state/src/lib.rs b/codex-rs/state/src/lib.rs index 4179cf0957..5564c36078 100644 --- a/codex-rs/state/src/lib.rs +++ b/codex-rs/state/src/lib.rs @@ -31,7 +31,6 @@ pub use model::Stage1Output; pub use model::ThreadMetadata; pub use model::ThreadMetadataBuilder; pub use model::ThreadsPage; -pub use runtime::PendingScopeConsolidation; pub use runtime::Phase2JobClaimOutcome; pub use runtime::STATE_DB_FILENAME; pub use runtime::STATE_DB_VERSION; diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index 6dd6347740..21f55d068d 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -39,9 +39,6 @@ use uuid::Uuid; pub const STATE_DB_FILENAME: &str = "state"; pub const STATE_DB_VERSION: u32 = 4; -const MEMORY_SCOPE_KIND_USER: &str = "user"; -const MEMORY_SCOPE_KEY_USER: &str = "user"; - const METRIC_DB_INIT: &str = "codex.db.init"; mod memory; @@ -86,27 +83,18 @@ pub struct Stage1StartupClaimParams<'a> { pub lease_seconds: i64, } -/// Scope row used to queue phase-2 consolidation work. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct PendingScopeConsolidation { - /// Scope family (`user`). - pub scope_kind: String, - /// Scope identifier keyed by `scope_kind`. - pub scope_key: String, -} - /// Result of trying to claim a phase-2 consolidation job. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Phase2JobClaimOutcome { - /// The caller owns the scope and should spawn consolidation. + /// The caller owns the global lock and should spawn consolidation. Claimed { ownership_token: String, /// Snapshot of `input_watermark` at claim time. input_watermark: i64, }, - /// The scope is not pending consolidation (or is already up to date). + /// The global job is not pending consolidation (or is already up to date). SkippedNotDirty, - /// Another worker currently owns a fresh lease for this scope. + /// Another worker currently owns a fresh global consolidation lease. SkippedRunning, } @@ -926,7 +914,6 @@ fn push_thread_order_and_limit( #[cfg(test)] mod tests { - use super::PendingScopeConsolidation; use super::Phase2JobClaimOutcome; use super::STATE_DB_FILENAME; use super::STATE_DB_VERSION; @@ -1463,7 +1450,7 @@ WHERE kind = 'memory_stage1' } #[tokio::test] - async fn phase2_consolidation_jobs_rerun_when_watermark_advances() { + async fn phase2_global_consolidation_reruns_when_watermark_advances() { let codex_home = unique_temp_dir(); let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) .await @@ -1472,24 +1459,12 @@ WHERE kind = 'memory_stage1' let owner = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner id"); runtime - .enqueue_scope_consolidation("user", "user", 100) + .enqueue_global_consolidation(100) .await - .expect("enqueue scope"); - - let scopes = runtime - .list_pending_scope_consolidations(10) - .await - .expect("list pending"); - assert_eq!( - scopes, - vec![PendingScopeConsolidation { - scope_kind: "user".to_string(), - scope_key: "user".to_string(), - }] - ); + .expect("enqueue global consolidation"); let claim = runtime - .try_claim_phase2_job("user", "user", owner, 3600) + .try_claim_global_phase2_job(owner, 3600) .await .expect("claim phase2"); let (ownership_token, input_watermark) = match claim { @@ -1501,30 +1476,25 @@ WHERE kind = 'memory_stage1' }; assert!( runtime - .mark_phase2_job_succeeded( - "user", - "user", - ownership_token.as_str(), - input_watermark, - ) + .mark_global_phase2_job_succeeded(ownership_token.as_str(), input_watermark) .await .expect("mark phase2 succeeded"), "phase2 success should finalize for current token" ); let claim_up_to_date = runtime - .try_claim_phase2_job("user", "user", owner, 3600) + .try_claim_global_phase2_job(owner, 3600) .await .expect("claim phase2 up-to-date"); assert_eq!(claim_up_to_date, Phase2JobClaimOutcome::SkippedNotDirty); runtime - .enqueue_scope_consolidation("user", "user", 101) + .enqueue_global_consolidation(101) .await - .expect("enqueue scope again"); + .expect("enqueue global consolidation again"); let claim_rerun = runtime - .try_claim_phase2_job("user", "user", owner, 3600) + .try_claim_global_phase2_job(owner, 3600) .await .expect("claim phase2 rerun"); assert!( @@ -1536,7 +1506,7 @@ WHERE kind = 'memory_stage1' } #[tokio::test] - async fn list_stage1_outputs_for_user_scope_returns_latest_outputs() { + async fn list_stage1_outputs_for_global_returns_latest_outputs() { let codex_home = unique_temp_dir(); let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) .await @@ -1607,9 +1577,9 @@ WHERE kind = 'memory_stage1' ); let outputs = runtime - .list_stage1_outputs_for_scope("user", "user", 10) + .list_stage1_outputs_for_global(10) .await - .expect("list stage1 outputs for user scope"); + .expect("list stage1 outputs for global"); assert_eq!(outputs.len(), 2); assert_eq!(outputs[0].thread_id, thread_id_b); assert_eq!(outputs[0].summary, "summary b"); @@ -1620,7 +1590,7 @@ WHERE kind = 'memory_stage1' } #[tokio::test] - async fn mark_stage1_job_succeeded_enqueues_single_user_scope() { + async fn mark_stage1_job_succeeded_enqueues_global_consolidation() { let codex_home = unique_temp_dir(); let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) .await @@ -1679,106 +1649,116 @@ WHERE kind = 'memory_stage1' "stage1 success should persist output for thread b" ); - let pending_scopes = runtime - .list_pending_scope_consolidations(10) + let claim = runtime + .try_claim_global_phase2_job(owner, 3600) .await - .expect("list pending scopes"); - assert_eq!( - pending_scopes, - vec![PendingScopeConsolidation { - scope_kind: "user".to_string(), - scope_key: "user".to_string(), - }] - ); + .expect("claim global consolidation"); + let input_watermark = match claim { + Phase2JobClaimOutcome::Claimed { + input_watermark, .. + } => input_watermark, + other => panic!("unexpected global consolidation claim outcome: {other:?}"), + }; + assert_eq!(input_watermark, 101); let _ = tokio::fs::remove_dir_all(codex_home).await; } #[tokio::test] - async fn list_pending_scope_consolidations_omits_unclaimable_jobs() { + async fn phase2_global_lock_allows_only_one_fresh_runner() { let codex_home = unique_temp_dir(); let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) .await .expect("initialize runtime"); - let owner = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner id"); runtime - .enqueue_scope_consolidation("user", "scope-running", 200) + .enqueue_global_consolidation(200) .await - .expect("enqueue running scope"); - runtime - .enqueue_scope_consolidation("user", "scope-backoff", 199) - .await - .expect("enqueue backoff scope"); - runtime - .enqueue_scope_consolidation("user", "scope-exhausted", 198) - .await - .expect("enqueue exhausted scope"); - runtime - .enqueue_scope_consolidation("user", "scope-claimable-a", 90) - .await - .expect("enqueue claimable scope a"); - runtime - .enqueue_scope_consolidation("user", "scope-claimable-b", 89) - .await - .expect("enqueue claimable scope b"); + .expect("enqueue global consolidation"); + + let owner_a = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner a"); + let owner_b = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner b"); let running_claim = runtime - .try_claim_phase2_job("user", "scope-running", owner, 3600) + .try_claim_global_phase2_job(owner_a, 3600) .await - .expect("claim running scope"); + .expect("claim global lock"); assert!( matches!(running_claim, Phase2JobClaimOutcome::Claimed { .. }), - "scope-running should be claimed" + "first owner should claim global lock" ); - let backoff_claim = runtime - .try_claim_phase2_job("user", "scope-backoff", owner, 3600) + let second_claim = runtime + .try_claim_global_phase2_job(owner_b, 3600) .await - .expect("claim backoff scope"); - let backoff_token = match backoff_claim { + .expect("claim global lock from second owner"); + assert_eq!(second_claim, Phase2JobClaimOutcome::SkippedRunning); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + + #[tokio::test] + async fn phase2_global_lock_stale_lease_allows_takeover() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + runtime + .enqueue_global_consolidation(300) + .await + .expect("enqueue global consolidation"); + + let owner_a = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner a"); + let owner_b = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner b"); + + let initial_claim = runtime + .try_claim_global_phase2_job(owner_a, 3600) + .await + .expect("claim initial global lock"); + let token_a = match initial_claim { Phase2JobClaimOutcome::Claimed { ownership_token, .. } => ownership_token, - other => panic!("unexpected backoff claim outcome: {other:?}"), + other => panic!("unexpected initial claim outcome: {other:?}"), }; - assert!( - runtime - .mark_phase2_job_failed( - "user", - "scope-backoff", - backoff_token.as_str(), - "temporary failure", - 3600, - ) - .await - .expect("mark backoff scope failed"), - "backoff scope should transition to retry backoff" - ); - sqlx::query("UPDATE jobs SET retry_remaining = 0 WHERE kind = ? AND job_key = ?") - .bind("memory_consolidate_user") - .bind("scope-exhausted") + sqlx::query("UPDATE jobs SET lease_until = ? WHERE kind = ? AND job_key = ?") + .bind(Utc::now().timestamp() - 1) + .bind("memory_consolidate_global") + .bind("global") .execute(runtime.pool.as_ref()) .await - .expect("set exhausted scope retries to zero"); + .expect("expire global consolidation lease"); - let pending = runtime - .list_pending_scope_consolidations(2) + let takeover_claim = runtime + .try_claim_global_phase2_job(owner_b, 3600) .await - .expect("list pending scopes"); + .expect("claim stale global lock"); + let (token_b, input_watermark) = match takeover_claim { + Phase2JobClaimOutcome::Claimed { + ownership_token, + input_watermark, + } => (ownership_token, input_watermark), + other => panic!("unexpected takeover claim outcome: {other:?}"), + }; + assert_ne!(token_a, token_b); + assert_eq!(input_watermark, 300); + assert_eq!( - pending, - vec![ - PendingScopeConsolidation { - scope_kind: "user".to_string(), - scope_key: "scope-claimable-a".to_string(), - }, - PendingScopeConsolidation { - scope_kind: "user".to_string(), - scope_key: "scope-claimable-b".to_string(), - }, - ] + runtime + .mark_global_phase2_job_succeeded(token_a.as_str(), 300) + .await + .expect("mark stale owner success result"), + false, + "stale owner should lose finalization ownership after takeover" + ); + assert!( + runtime + .mark_global_phase2_job_succeeded(token_b.as_str(), 300) + .await + .expect("mark takeover owner success"), + "takeover owner should finalize consolidation" ); let _ = tokio::fs::remove_dir_all(codex_home).await; diff --git a/codex-rs/state/src/runtime/memory.rs b/codex-rs/state/src/runtime/memory.rs index f25978a063..65ce9b08f7 100644 --- a/codex-rs/state/src/runtime/memory.rs +++ b/codex-rs/state/src/runtime/memory.rs @@ -8,26 +8,11 @@ use sqlx::QueryBuilder; use sqlx::Sqlite; const JOB_KIND_MEMORY_STAGE1: &str = "memory_stage1"; -const JOB_KIND_MEMORY_CONSOLIDATE_USER: &str = "memory_consolidate_user"; +const JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL: &str = "memory_consolidate_global"; +const MEMORY_CONSOLIDATION_JOB_KEY: &str = "global"; const DEFAULT_RETRY_REMAINING: i64 = 3; -fn job_kind_for_scope(scope_kind: &str) -> Option<&'static str> { - if scope_kind == MEMORY_SCOPE_KIND_USER { - Some(JOB_KIND_MEMORY_CONSOLIDATE_USER) - } else { - None - } -} - -fn scope_kind_for_job_kind(job_kind: &str) -> Option<&'static str> { - if job_kind == JOB_KIND_MEMORY_CONSOLIDATE_USER { - Some(MEMORY_SCOPE_KIND_USER) - } else { - None - } -} - impl StateRuntime { pub async fn claim_stage1_jobs_for_startup( &self, @@ -145,33 +130,26 @@ WHERE thread_id = ? .transpose() } - pub async fn list_stage1_outputs_for_scope( + pub async fn list_stage1_outputs_for_global( &self, - scope_kind: &str, - _scope_key: &str, n: usize, ) -> anyhow::Result> { if n == 0 { return Ok(Vec::new()); } - let rows = match scope_kind { - MEMORY_SCOPE_KIND_USER => { - sqlx::query( - r#" + let rows = sqlx::query( + r#" SELECT so.thread_id, so.source_updated_at, so.raw_memory, so.summary, so.generated_at FROM stage1_outputs AS so JOIN threads AS t ON t.id = so.thread_id ORDER BY so.source_updated_at DESC, so.thread_id DESC LIMIT ? - "#, - ) - .bind(n as i64) - .fetch_all(self.pool.as_ref()) - .await? - } - _ => return Ok(Vec::new()), - }; + "#, + ) + .bind(n as i64) + .fetch_all(self.pool.as_ref()) + .await?; rows.into_iter() .map(|row| Stage1OutputRow::try_from_row(&row).and_then(Stage1Output::try_from)) @@ -408,13 +386,7 @@ WHERE excluded.source_updated_at >= stage1_outputs.source_updated_at .execute(&mut *tx) .await?; - enqueue_scope_consolidation_with_executor( - &mut *tx, - MEMORY_SCOPE_KIND_USER, - MEMORY_SCOPE_KEY_USER, - source_updated_at, - ) - .await?; + enqueue_global_consolidation_with_executor(&mut *tx, source_updated_at).await?; tx.commit().await?; Ok(true) @@ -458,77 +430,16 @@ WHERE kind = ? AND job_key = ? Ok(rows_affected > 0) } - pub async fn enqueue_scope_consolidation( - &self, - scope_kind: &str, - scope_key: &str, - input_watermark: i64, - ) -> anyhow::Result<()> { - enqueue_scope_consolidation_with_executor( - self.pool.as_ref(), - scope_kind, - scope_key, - input_watermark, - ) - .await + pub async fn enqueue_global_consolidation(&self, input_watermark: i64) -> anyhow::Result<()> { + enqueue_global_consolidation_with_executor(self.pool.as_ref(), input_watermark).await } - pub async fn list_pending_scope_consolidations( + /// Try to claim the global phase-2 consolidation job. + pub async fn try_claim_global_phase2_job( &self, - limit: usize, - ) -> anyhow::Result> { - if limit == 0 { - return Ok(Vec::new()); - } - let now = Utc::now().timestamp(); - - let rows = sqlx::query( - r#" -SELECT kind, job_key -FROM jobs -WHERE kind = ? - AND input_watermark IS NOT NULL - AND input_watermark > COALESCE(last_success_watermark, 0) - AND retry_remaining > 0 - AND (retry_at IS NULL OR retry_at <= ?) - AND (status != 'running' OR lease_until IS NULL OR lease_until <= ?) -ORDER BY input_watermark DESC, kind ASC, job_key ASC -LIMIT ? - "#, - ) - .bind(JOB_KIND_MEMORY_CONSOLIDATE_USER) - .bind(now) - .bind(now) - .bind(limit as i64) - .fetch_all(self.pool.as_ref()) - .await?; - - Ok(rows - .into_iter() - .filter_map(|row| { - let kind: String = row.try_get("kind").ok()?; - let scope_kind = scope_kind_for_job_kind(&kind)?; - let scope_key: String = row.try_get("job_key").ok()?; - Some(PendingScopeConsolidation { - scope_kind: scope_kind.to_string(), - scope_key, - }) - }) - .collect::>()) - } - - /// Try to claim a phase-2 consolidation job for `(scope_kind, scope_key)`. - pub async fn try_claim_phase2_job( - &self, - scope_kind: &str, - scope_key: &str, worker_id: ThreadId, lease_seconds: i64, ) -> anyhow::Result { - let Some(job_kind) = job_kind_for_scope(scope_kind) else { - return Ok(Phase2JobClaimOutcome::SkippedNotDirty); - }; - let now = Utc::now().timestamp(); let lease_until = now.saturating_add(lease_seconds.max(0)); let ownership_token = Uuid::new_v4().to_string(); @@ -543,8 +454,8 @@ FROM jobs WHERE kind = ? AND job_key = ? "#, ) - .bind(job_kind) - .bind(scope_key) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .bind(MEMORY_CONSOLIDATION_JOB_KEY) .fetch_optional(&mut *tx) .await?; @@ -593,6 +504,7 @@ SET retry_at = NULL, last_error = NULL WHERE kind = ? AND job_key = ? + AND input_watermark > COALESCE(last_success_watermark, 0) AND (status != 'running' OR lease_until IS NULL OR lease_until <= ?) AND (retry_at IS NULL OR retry_at <= ?) AND retry_remaining > 0 @@ -602,8 +514,8 @@ WHERE kind = ? AND job_key = ? .bind(ownership_token.as_str()) .bind(now) .bind(lease_until) - .bind(job_kind) - .bind(scope_key) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .bind(MEMORY_CONSOLIDATION_JOB_KEY) .bind(now) .bind(now) .execute(&mut *tx) @@ -621,17 +533,11 @@ WHERE kind = ? AND job_key = ? } } - pub async fn heartbeat_phase2_job( + pub async fn heartbeat_global_phase2_job( &self, - scope_kind: &str, - scope_key: &str, ownership_token: &str, lease_seconds: i64, ) -> anyhow::Result { - let Some(job_kind) = job_kind_for_scope(scope_kind) else { - return Ok(false); - }; - let now = Utc::now().timestamp(); let lease_until = now.saturating_add(lease_seconds.max(0)); let rows_affected = sqlx::query( @@ -643,8 +549,8 @@ WHERE kind = ? AND job_key = ? "#, ) .bind(lease_until) - .bind(job_kind) - .bind(scope_key) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .bind(MEMORY_CONSOLIDATION_JOB_KEY) .bind(ownership_token) .execute(self.pool.as_ref()) .await? @@ -653,17 +559,11 @@ WHERE kind = ? AND job_key = ? Ok(rows_affected > 0) } - pub async fn mark_phase2_job_succeeded( + pub async fn mark_global_phase2_job_succeeded( &self, - scope_kind: &str, - scope_key: &str, ownership_token: &str, completed_watermark: i64, ) -> anyhow::Result { - let Some(job_kind) = job_kind_for_scope(scope_kind) else { - return Ok(false); - }; - let now = Utc::now().timestamp(); let rows_affected = sqlx::query( r#" @@ -680,8 +580,8 @@ WHERE kind = ? AND job_key = ? ) .bind(now) .bind(completed_watermark) - .bind(job_kind) - .bind(scope_key) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .bind(MEMORY_CONSOLIDATION_JOB_KEY) .bind(ownership_token) .execute(self.pool.as_ref()) .await? @@ -690,18 +590,12 @@ WHERE kind = ? AND job_key = ? Ok(rows_affected > 0) } - pub async fn mark_phase2_job_failed( + pub async fn mark_global_phase2_job_failed( &self, - scope_kind: &str, - scope_key: &str, ownership_token: &str, failure_reason: &str, retry_delay_seconds: i64, ) -> anyhow::Result { - let Some(job_kind) = job_kind_for_scope(scope_kind) else { - return Ok(false); - }; - let now = Utc::now().timestamp(); let retry_at = now.saturating_add(retry_delay_seconds.max(0)); let rows_affected = sqlx::query( @@ -721,8 +615,8 @@ WHERE kind = ? AND job_key = ? .bind(now) .bind(retry_at) .bind(failure_reason) - .bind(job_kind) - .bind(scope_key) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .bind(MEMORY_CONSOLIDATION_JOB_KEY) .bind(ownership_token) .execute(self.pool.as_ref()) .await? @@ -732,19 +626,13 @@ WHERE kind = ? AND job_key = ? } } -async fn enqueue_scope_consolidation_with_executor<'e, E>( +async fn enqueue_global_consolidation_with_executor<'e, E>( executor: E, - scope_kind: &str, - scope_key: &str, input_watermark: i64, ) -> anyhow::Result<()> where E: Executor<'e, Database = Sqlite>, { - let Some(job_kind) = job_kind_for_scope(scope_kind) else { - return Ok(()); - }; - sqlx::query( r#" INSERT INTO jobs ( @@ -775,8 +663,8 @@ ON CONFLICT(kind, job_key) DO UPDATE SET input_watermark = max(COALESCE(jobs.input_watermark, 0), excluded.input_watermark) "#, ) - .bind(job_kind) - .bind(scope_key) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .bind(MEMORY_CONSOLIDATION_JOB_KEY) .bind(DEFAULT_RETRY_REMAINING) .bind(input_watermark) .execute(executor) From 1d47927aa06a4d4a0d15826dd04ceeee0f7c665a Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 10 Feb 2026 15:13:52 -0800 Subject: [PATCH 2/7] Enable SOCKS defaults for common local network proxy use cases (#11362) ## Summary - enable local-use defaults in network proxy settings: SOCKS5 on, SOCKS5 UDP on, upstream proxying on, and local binding on - add a regression test that asserts the full `NetworkProxySettings::default()` baseline - Fixed managed listener reservation behavior. Before: we always reserved a loopback SOCKS listener, even when enable_socks5 = false. Now: SOCKS listener is only reserved when SOCKS is enabled. - Fixed /debug-config env output for SOCKS-disabled sessions. ALL_PROXY now shows the HTTP proxy URL when SOCKS is disabled (instead of incorrectly showing socks5h://...). ## Validation - just fmt - cargo test -p codex-network-proxy - cargo clippy -p codex-network-proxy --all-targets --- .../core/src/config/network_proxy_spec.rs | 4 + codex-rs/network-proxy/README.md | 14 ++-- codex-rs/network-proxy/src/config.rs | 54 ++++++++++++-- codex-rs/network-proxy/src/proxy.rs | 74 +++++++++++++++---- codex-rs/tui/src/debug_config.rs | 35 ++++++++- 5 files changed, 152 insertions(+), 29 deletions(-) diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index e50a30d885..ee3e8ebed1 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -68,6 +68,10 @@ impl NetworkProxySpec { host_and_port_from_network_addr(&self.config.network.proxy_url, 3128) } + pub fn socks_enabled(&self) -> bool { + self.config.network.enable_socks5 + } + pub(crate) fn from_constraints( _config_layer_stack: &config::ConfigLayerStack, requirements: NetworkConstraints, diff --git a/codex-rs/network-proxy/README.md b/codex-rs/network-proxy/README.md index 3a8d20eb80..7ffed9008d 100644 --- a/codex-rs/network-proxy/README.md +++ b/codex-rs/network-proxy/README.md @@ -3,7 +3,7 @@ `codex-network-proxy` is Codex's local network policy enforcement proxy. It runs: - an HTTP proxy (default `127.0.0.1:3128`) -- an optional SOCKS5 proxy (default `127.0.0.1:8081`, disabled by default) +- a SOCKS5 proxy (default `127.0.0.1:8081`, enabled by default) - an admin HTTP API (default `127.0.0.1:8080`) It enforces an allow/deny policy and a "limited" mode intended for read-only network access. @@ -21,14 +21,14 @@ Example config: enabled = true proxy_url = "http://127.0.0.1:3128" admin_url = "http://127.0.0.1:8080" -# Optional SOCKS5 listener (disabled by default). -enable_socks5 = false +# SOCKS5 listener (enabled by default). +enable_socks5 = true socks_url = "http://127.0.0.1:8081" -enable_socks5_udp = false +enable_socks5_udp = true # When `enabled` is false, the proxy no-ops and does not bind listeners. # When true, respect HTTP(S)_PROXY/ALL_PROXY for upstream requests (HTTP(S) proxies only), # including CONNECT tunnels in full mode. -allow_upstream_proxy = false +allow_upstream_proxy = true # By default, non-loopback binds are clamped to loopback for safety. # If you want to expose these listeners beyond localhost, you must opt in explicitly. dangerously_allow_non_loopback_proxy = false @@ -37,13 +37,13 @@ mode = "full" # default when unset; use "limited" for read-only mode # Hosts must match the allowlist (unless denied). # If `allowed_domains` is empty, the proxy blocks requests until an allowlist is configured. -allowed_domains = ["*.openai.com"] +allowed_domains = ["*.openai.com", "localhost", "127.0.0.1", "::1"] denied_domains = ["evil.example"] # If false, local/private networking is rejected. Explicit allowlisting of local IP literals # (or `localhost`) is required to permit them. # Hostnames that resolve to local/private IPs are still blocked even if allowlisted. -allow_local_binding = false +allow_local_binding = true # macOS-only: allows proxying to a unix socket when request includes `x-unix-socket: /path`. allow_unix_sockets = ["/tmp/example.sock"] diff --git a/codex-rs/network-proxy/src/config.rs b/codex-rs/network-proxy/src/config.rs index 505fa96d32..31197a1c96 100644 --- a/codex-rs/network-proxy/src/config.rs +++ b/codex-rs/network-proxy/src/config.rs @@ -15,6 +15,7 @@ pub struct NetworkProxyConfig { } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(default)] pub struct NetworkProxySettings { #[serde(default)] pub enabled: bool, @@ -22,13 +23,10 @@ pub struct NetworkProxySettings { pub proxy_url: String, #[serde(default = "default_admin_url")] pub admin_url: String, - #[serde(default)] pub enable_socks5: bool, #[serde(default = "default_socks_url")] pub socks_url: String, - #[serde(default)] pub enable_socks5_udp: bool, - #[serde(default)] pub allow_upstream_proxy: bool, #[serde(default)] pub dangerously_allow_non_loopback_proxy: bool, @@ -42,7 +40,6 @@ pub struct NetworkProxySettings { pub denied_domains: Vec, #[serde(default)] pub allow_unix_sockets: Vec, - #[serde(default)] pub allow_local_binding: bool, } @@ -52,17 +49,17 @@ impl Default for NetworkProxySettings { enabled: false, proxy_url: default_proxy_url(), admin_url: default_admin_url(), - enable_socks5: false, + enable_socks5: true, socks_url: default_socks_url(), - enable_socks5_udp: false, - allow_upstream_proxy: false, + enable_socks5_udp: true, + allow_upstream_proxy: true, dangerously_allow_non_loopback_proxy: false, dangerously_allow_non_loopback_admin: false, mode: NetworkMode::default(), allowed_domains: Vec::new(), denied_domains: Vec::new(), allow_unix_sockets: Vec::new(), - allow_local_binding: false, + allow_local_binding: true, } } } @@ -329,6 +326,47 @@ mod tests { use pretty_assertions::assert_eq; + #[test] + fn network_proxy_settings_default_matches_local_use_baseline() { + assert_eq!( + NetworkProxySettings::default(), + NetworkProxySettings { + enabled: false, + proxy_url: "http://127.0.0.1:3128".to_string(), + admin_url: "http://127.0.0.1:8080".to_string(), + enable_socks5: true, + socks_url: "http://127.0.0.1:8081".to_string(), + enable_socks5_udp: true, + allow_upstream_proxy: true, + dangerously_allow_non_loopback_proxy: false, + dangerously_allow_non_loopback_admin: false, + mode: NetworkMode::Full, + allowed_domains: Vec::new(), + denied_domains: Vec::new(), + allow_unix_sockets: Vec::new(), + allow_local_binding: true, + } + ); + } + + #[test] + fn partial_network_config_uses_struct_defaults_for_missing_fields() { + let config: NetworkProxyConfig = serde_json::from_str( + r#"{ + "network": { + "enabled": true + } + }"#, + ) + .unwrap(); + let expected = NetworkProxySettings { + enabled: true, + ..NetworkProxySettings::default() + }; + + assert_eq!(config.network, expected); + } + #[test] fn parse_host_port_defaults_for_empty_string() { assert!(parse_host_port("", 1234).is_err()); diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index 0a95e44608..566ac0da25 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -29,10 +29,10 @@ struct ReservedListeners { } impl ReservedListeners { - fn new(http: StdTcpListener, socks: StdTcpListener, admin: StdTcpListener) -> Self { + fn new(http: StdTcpListener, socks: Option, admin: StdTcpListener) -> Self { Self { http: Mutex::new(Some(http)), - socks: Mutex::new(Some(socks)), + socks: Mutex::new(socks), admin: Mutex::new(Some(admin)), } } @@ -133,15 +133,20 @@ impl NetworkProxyBuilder { let current_cfg = state.current_cfg().await?; let (requested_http_addr, requested_socks_addr, requested_admin_addr, reserved_listeners) = if self.managed_by_codex { + let runtime = config::resolve_runtime(¤t_cfg)?; let (http_listener, socks_listener, admin_listener) = - reserve_loopback_ephemeral_listeners() + reserve_loopback_ephemeral_listeners(current_cfg.network.enable_socks5) .context("reserve managed loopback proxy listeners")?; let http_addr = http_listener .local_addr() .context("failed to read reserved HTTP proxy address")?; - let socks_addr = socks_listener - .local_addr() - .context("failed to read reserved SOCKS5 proxy address")?; + let socks_addr = if let Some(socks_listener) = socks_listener.as_ref() { + socks_listener + .local_addr() + .context("failed to read reserved SOCKS5 proxy address")? + } else { + runtime.socks_addr + }; let admin_addr = admin_listener .local_addr() .context("failed to read reserved admin API address")?; @@ -186,13 +191,19 @@ impl NetworkProxyBuilder { } } -fn reserve_loopback_ephemeral_listeners() -> Result<(StdTcpListener, StdTcpListener, StdTcpListener)> -{ - Ok(( - reserve_loopback_ephemeral_listener().context("reserve HTTP proxy listener")?, - reserve_loopback_ephemeral_listener().context("reserve SOCKS5 proxy listener")?, - reserve_loopback_ephemeral_listener().context("reserve admin API listener")?, - )) +fn reserve_loopback_ephemeral_listeners( + reserve_socks_listener: bool, +) -> Result<(StdTcpListener, Option, StdTcpListener)> { + let http_listener = + reserve_loopback_ephemeral_listener().context("reserve HTTP proxy listener")?; + let socks_listener = if reserve_socks_listener { + Some(reserve_loopback_ephemeral_listener().context("reserve SOCKS5 proxy listener")?) + } else { + None + }; + let admin_listener = + reserve_loopback_ephemeral_listener().context("reserve admin API listener")?; + Ok((http_listener, socks_listener, admin_listener)) } fn reserve_loopback_ephemeral_listener() -> Result { @@ -612,6 +623,43 @@ mod tests { ); } + #[tokio::test] + async fn managed_proxy_builder_does_not_reserve_socks_listener_when_disabled() { + let settings = NetworkProxySettings { + enable_socks5: false, + socks_url: "http://127.0.0.1:43129".to_string(), + ..NetworkProxySettings::default() + }; + let state = Arc::new(network_proxy_state_for_policy(settings)); + let proxy = match NetworkProxy::builder().state(state).build().await { + Ok(proxy) => proxy, + Err(err) => { + if err + .chain() + .any(|cause| cause.to_string().contains("Operation not permitted")) + { + return; + } + panic!("failed to build managed proxy: {err:#}"); + } + }; + + assert!(proxy.http_addr.ip().is_loopback()); + assert!(proxy.admin_addr.ip().is_loopback()); + assert_eq!( + proxy.socks_addr, + "127.0.0.1:43129".parse::().unwrap() + ); + assert!( + proxy + .reserved_listeners + .as_ref() + .expect("managed builder should reserve listeners") + .take_socks() + .is_none() + ); + } + #[test] fn proxy_url_env_value_resolves_lowercase_aliases() { let mut env = HashMap::new(); diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index b5edeeb232..681d9c3a9a 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -27,14 +27,30 @@ pub(crate) fn new_debug_config_output( socks_addr, admin_addr, } = proxy; + let all_proxy = session_all_proxy_url( + http_addr, + socks_addr, + config + .network + .as_ref() + .is_some_and(codex_core::config::NetworkProxySpec::socks_enabled), + ); lines.push(format!(" - HTTP_PROXY = http://{http_addr}").into()); - lines.push(format!(" - ALL_PROXY = socks5h://{socks_addr}").into()); + lines.push(format!(" - ALL_PROXY = {all_proxy}").into()); lines.push(format!(" - ADMIN_PROXY = http://{admin_addr}").into()); } PlainHistoryCell::new(lines) } +fn session_all_proxy_url(http_addr: &str, socks_addr: &str, socks_enabled: bool) -> String { + if socks_enabled { + format!("socks5h://{socks_addr}") + } else { + format!("http://{http_addr}") + } +} + fn render_debug_config_lines(stack: &ConfigLayerStack) -> Vec> { let mut lines = vec!["/debug-config".magenta().into(), "".into()]; @@ -288,6 +304,7 @@ fn format_network_constraints(network: &NetworkConstraints) -> String { #[cfg(test)] mod tests { use super::render_debug_config_lines; + use super::session_all_proxy_url; use codex_app_server_protocol::ConfigLayerSource; use codex_core::config::Constrained; use codex_core::config_loader::ConfigLayerEntry; @@ -503,4 +520,20 @@ mod tests { rendered.contains("allowed_web_search_modes: disabled (source: cloud requirements)") ); } + + #[test] + fn session_all_proxy_url_uses_socks_when_enabled() { + assert_eq!( + session_all_proxy_url("127.0.0.1:3128", "127.0.0.1:8081", true), + "socks5h://127.0.0.1:8081".to_string() + ); + } + + #[test] + fn session_all_proxy_url_uses_http_when_socks_disabled() { + assert_eq!( + session_all_proxy_url("127.0.0.1:3128", "127.0.0.1:8081", false), + "http://127.0.0.1:3128".to_string() + ); + } } From 34fb4b6e6353868de8ee7d5efa4ba2b90df0ebb1 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Tue, 10 Feb 2026 15:19:55 -0800 Subject: [PATCH 3/7] ci: fall back to local Bazel on forks without BuildBuddy key (#11359) ## Summary - detect whether BUILDBUDDY_API_KEY is present in Bazel CI - keep existing remote BuildBuddy path when key is available - add a local fallback path for fork PRs without secrets by clearing remote cache/executor/BES endpoints - document each fallback flag inline with links to Bazel docs ## Testing - ruby -e 'require "yaml"; YAML.load_file(".github/workflows/bazel.yml"); puts "ok"' - verified Bazel docs/flag references used in workflow comments --- .github/workflows/bazel.yml | 45 ++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index b5e91d7f22..b1ae82318c 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -103,9 +103,42 @@ jobs: CODEX_BWRAP_ENABLE_FFI: ${{ contains(matrix.target, 'unknown-linux') && '1' || '0' }} shell: bash run: | - bazel $BAZEL_STARTUP_ARGS --bazelrc=.github/workflows/ci.bazelrc test //... \ - --build_metadata=REPO_URL=https://github.com/openai/codex.git \ - --build_metadata=COMMIT_SHA=$(git rev-parse HEAD) \ - --build_metadata=ROLE=CI \ - --build_metadata=VISIBILITY=PUBLIC \ - "--remote_header=x-buildbuddy-api-key=$BUILDBUDDY_API_KEY" + bazel_args=( + test + //... + --build_metadata=REPO_URL=https://github.com/openai/codex.git + --build_metadata=COMMIT_SHA=$(git rev-parse HEAD) + --build_metadata=ROLE=CI + --build_metadata=VISIBILITY=PUBLIC + ) + + if [[ -n "${BUILDBUDDY_API_KEY:-}" ]]; then + echo "BuildBuddy API key is available; using remote Bazel configuration." + bazel $BAZEL_STARTUP_ARGS \ + --bazelrc=.github/workflows/ci.bazelrc \ + "${bazel_args[@]}" \ + "--remote_header=x-buildbuddy-api-key=$BUILDBUDDY_API_KEY" + else + echo "BuildBuddy API key is not available; using local Bazel configuration." + # Keep fork/community PRs on Bazel but disable remote services that are + # configured in .bazelrc and require auth. + # + # Flag docs: + # - Command-line reference: https://bazel.build/reference/command-line-reference + # - Remote caching overview: https://bazel.build/remote/caching + # - Remote execution overview: https://bazel.build/remote/rbe + # - Build Event Protocol overview: https://bazel.build/remote/bep + # + # --noexperimental_remote_repo_contents_cache: + # disable remote repo contents cache enabled in .bazelrc startup options. + # https://bazel.build/reference/command-line-reference#startup_options-flag--experimental_remote_repo_contents_cache + # --remote_cache= and --remote_executor=: + # clear remote cache/execution endpoints configured in .bazelrc. + # https://bazel.build/reference/command-line-reference#common_options-flag--remote_cache + # https://bazel.build/reference/command-line-reference#common_options-flag--remote_executor + bazel $BAZEL_STARTUP_ARGS \ + --noexperimental_remote_repo_contents_cache \ + "${bazel_args[@]}" \ + --remote_cache= \ + --remote_executor= + fi From 2c9be54c9a1d1229d7923f2ad8cd557681746fc4 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 10 Feb 2026 23:22:55 +0000 Subject: [PATCH 4/7] feat: mem v2 - PR5 (#11372) --- codex-rs/core/src/memories/layout.rs | 83 -------------- codex-rs/core/src/memories/mod.rs | 38 ++++++- codex-rs/core/src/memories/stage_one.rs | 10 +- .../core/src/memories/startup/dispatch.rs | 4 +- codex-rs/core/src/memories/startup/extract.rs | 2 +- codex-rs/core/src/memories/startup/mod.rs | 21 +--- .../memories/startup/{watch.rs => phase2.rs} | 0 codex-rs/core/src/memories/storage.rs | 31 +++--- codex-rs/core/src/memories/tests.rs | 55 ++------- codex-rs/core/src/memories/types.rs | 15 --- .../core/templates/memories/consolidation.md | 2 +- .../templates/memories/stage_one_input.md | 2 +- .../templates/memories/stage_one_system.md | 1 - ...d_stage1_outputs.sql => 0006_memories.sql} | 16 +-- .../state/migrations/0006_thread_memory.sql | 9 -- .../0009_memory_consolidation_locks.sql | 8 -- .../migrations/0010_memory_workflow_v2.sql | 85 -------------- codex-rs/state/src/lib.rs | 8 +- codex-rs/state/src/model/memories.rs | 105 ++++++++++++++++++ codex-rs/state/src/model/mod.rs | 10 +- codex-rs/state/src/model/stage1_output.rs | 56 ---------- codex-rs/state/src/runtime.rs | 61 ++-------- .../src/runtime/{memory.rs => memories.rs} | 19 ++-- 23 files changed, 208 insertions(+), 433 deletions(-) delete mode 100644 codex-rs/core/src/memories/layout.rs rename codex-rs/core/src/memories/startup/{watch.rs => phase2.rs} (100%) delete mode 100644 codex-rs/core/src/memories/types.rs rename codex-rs/state/migrations/{0011_generic_jobs_and_stage1_outputs.sql => 0006_memories.sql} (61%) delete mode 100644 codex-rs/state/migrations/0006_thread_memory.sql delete mode 100644 codex-rs/state/migrations/0009_memory_consolidation_locks.sql delete mode 100644 codex-rs/state/migrations/0010_memory_workflow_v2.sql create mode 100644 codex-rs/state/src/model/memories.rs delete mode 100644 codex-rs/state/src/model/stage1_output.rs rename codex-rs/state/src/runtime/{memory.rs => memories.rs} (97%) diff --git a/codex-rs/core/src/memories/layout.rs b/codex-rs/core/src/memories/layout.rs deleted file mode 100644 index 07a8bd4ec0..0000000000 --- a/codex-rs/core/src/memories/layout.rs +++ /dev/null @@ -1,83 +0,0 @@ -use std::path::Path; -use std::path::PathBuf; - -pub(super) const ROLLOUT_SUMMARIES_SUBDIR: &str = "rollout_summaries"; -pub(super) const RAW_MEMORIES_FILENAME: &str = "raw_memories.md"; -pub(super) const MEMORY_REGISTRY_FILENAME: &str = "MEMORY.md"; -pub(super) const LEGACY_CONSOLIDATED_FILENAME: &str = "consolidated.md"; -pub(super) const SKILLS_SUBDIR: &str = "skills"; -const LEGACY_USER_SUBDIR: &str = "user"; -const LEGACY_MEMORY_SUBDIR: &str = "memory"; - -/// Returns the shared on-disk memory root directory. -pub(super) fn memory_root(codex_home: &Path) -> PathBuf { - codex_home.join("memories") -} - -pub(super) fn rollout_summaries_dir(root: &Path) -> PathBuf { - root.join(ROLLOUT_SUMMARIES_SUBDIR) -} - -pub(super) fn raw_memories_file(root: &Path) -> PathBuf { - root.join(RAW_MEMORIES_FILENAME) -} - -/// Migrates legacy user memory contents into the shared root when no shared-root -/// phase artifacts exist yet. -pub(super) async fn migrate_legacy_user_memory_root_if_needed( - codex_home: &Path, -) -> std::io::Result<()> { - let root = memory_root(codex_home); - let legacy = legacy_user_memory_root(codex_home); - - if !tokio::fs::try_exists(&legacy).await? || global_root_has_phase_artifacts(&root).await? { - return Ok(()); - } - - copy_dir_contents_if_missing(&legacy, &root).await -} - -/// Ensures the phase-1 memory directory layout exists for the given root. -pub(super) async fn ensure_layout(root: &Path) -> std::io::Result<()> { - tokio::fs::create_dir_all(rollout_summaries_dir(root)).await -} - -fn legacy_user_memory_root(codex_home: &Path) -> PathBuf { - codex_home - .join("memories") - .join(LEGACY_USER_SUBDIR) - .join(LEGACY_MEMORY_SUBDIR) -} - -async fn global_root_has_phase_artifacts(root: &Path) -> std::io::Result { - if tokio::fs::try_exists(&rollout_summaries_dir(root)).await? - || tokio::fs::try_exists(&raw_memories_file(root)).await? - || tokio::fs::try_exists(&root.join(MEMORY_REGISTRY_FILENAME)).await? - || tokio::fs::try_exists(&root.join(LEGACY_CONSOLIDATED_FILENAME)).await? - || tokio::fs::try_exists(&root.join(SKILLS_SUBDIR)).await? - { - return Ok(true); - } - Ok(false) -} - -fn copy_dir_contents_if_missing<'a>( - src_dir: &'a Path, - dst_dir: &'a Path, -) -> futures::future::BoxFuture<'a, std::io::Result<()>> { - Box::pin(async move { - tokio::fs::create_dir_all(dst_dir).await?; - let mut dir = tokio::fs::read_dir(src_dir).await?; - while let Some(entry) = dir.next_entry().await? { - let src_path = entry.path(); - let dst_path = dst_dir.join(entry.file_name()); - let metadata = entry.metadata().await?; - if metadata.is_dir() { - copy_dir_contents_if_missing(&src_path, &dst_path).await?; - } else if metadata.is_file() && !tokio::fs::try_exists(&dst_path).await? { - tokio::fs::copy(&src_path, &dst_path).await?; - } - } - Ok(()) - }) -} diff --git a/codex-rs/core/src/memories/mod.rs b/codex-rs/core/src/memories/mod.rs index cfda3f47eb..10c50406a6 100644 --- a/codex-rs/core/src/memories/mod.rs +++ b/codex-rs/core/src/memories/mod.rs @@ -4,20 +4,26 @@ //! - Phase 1: select rollouts, extract stage-1 raw memories, persist stage-1 outputs, and enqueue consolidation. //! - Phase 2: claim a global consolidation lock, materialize consolidation inputs, and dispatch one consolidation agent. -mod layout; mod prompts; mod rollout; mod stage_one; mod startup; mod storage; mod text; -mod types; #[cfg(test)] mod tests; +use serde::Deserialize; +use std::path::Path; +use std::path::PathBuf; + /// Subagent source label used to identify consolidation tasks. const MEMORY_CONSOLIDATION_SUBAGENT_LABEL: &str = "memory_consolidation"; +const ROLLOUT_SUMMARIES_SUBDIR: &str = "rollout_summaries"; +const RAW_MEMORIES_FILENAME: &str = "raw_memories.md"; +const MEMORY_REGISTRY_FILENAME: &str = "MEMORY.md"; +const SKILLS_SUBDIR: &str = "skills"; /// Maximum number of rollout candidates processed per startup pass. const MAX_ROLLOUTS_PER_STARTUP: usize = 64; /// Concurrency cap for startup memory extraction and consolidation scheduling. @@ -39,6 +45,34 @@ const PHASE_TWO_JOB_RETRY_DELAY_SECONDS: i64 = 3_600; /// Heartbeat interval (seconds) for phase-2 running jobs. const PHASE_TWO_JOB_HEARTBEAT_SECONDS: u64 = 30; +/// Parsed stage-1 model output payload. +#[derive(Debug, Clone, Deserialize)] +#[serde(deny_unknown_fields)] +struct StageOneOutput { + /// Detailed markdown raw memory for a single rollout. + #[serde(rename = "raw_memory")] + raw_memory: String, + /// Compact summary line used for routing and indexing. + #[serde(rename = "rollout_summary")] + rollout_summary: String, +} + +fn memory_root(codex_home: &Path) -> PathBuf { + codex_home.join("memories") +} + +fn rollout_summaries_dir(root: &Path) -> PathBuf { + root.join(ROLLOUT_SUMMARIES_SUBDIR) +} + +fn raw_memories_file(root: &Path) -> PathBuf { + root.join(RAW_MEMORIES_FILENAME) +} + +async fn ensure_layout(root: &Path) -> std::io::Result<()> { + tokio::fs::create_dir_all(rollout_summaries_dir(root)).await +} + /// Starts the memory startup pipeline for eligible root sessions. /// /// This is the single entrypoint that `codex` uses to trigger memory startup. diff --git a/codex-rs/core/src/memories/stage_one.rs b/codex-rs/core/src/memories/stage_one.rs index c9f2f650e1..7f73fc2629 100644 --- a/codex-rs/core/src/memories/stage_one.rs +++ b/codex-rs/core/src/memories/stage_one.rs @@ -5,9 +5,9 @@ use regex::Regex; use serde_json::Value; use serde_json::json; +use super::StageOneOutput; use super::text::compact_whitespace; use super::text::truncate_text_for_storage; -use super::types::StageOneOutput; /// System prompt for stage-1 raw memory extraction. pub(super) const RAW_MEMORY_PROMPT: &str = @@ -28,7 +28,6 @@ pub(super) fn stage_one_output_schema() -> Value { json!({ "type": "object", "properties": { - "rollout_slug": { "type": "string" }, "rollout_summary": { "type": "string" }, "raw_memory": { "type": "string" } }, @@ -97,12 +96,6 @@ fn parse_json_object_loose(raw: &str) -> Result { fn normalize_stage_one_output(mut output: StageOneOutput) -> Result { output.raw_memory = output.raw_memory.trim().to_string(); output.rollout_summary = output.rollout_summary.trim().to_string(); - if let Some(slug) = output.rollout_slug.take() { - let slug = slug.trim(); - if !slug.is_empty() { - output.rollout_slug = Some(slug.to_string()); - } - } if output.raw_memory.is_empty() { return Err(CodexErr::InvalidRequest( @@ -195,7 +188,6 @@ mod tests { fn normalize_stage_one_output_redacts_and_compacts_summary() { let output = StageOneOutput { raw_memory: "Token: sk-abcdefghijklmnopqrstuvwxyz123456\nBearer abcdefghijklmnopqrstuvwxyz012345".to_string(), - rollout_slug: None, rollout_summary: "password = mysecret123456\n\nsmall".to_string(), }; diff --git a/codex-rs/core/src/memories/startup/dispatch.rs b/codex-rs/core/src/memories/startup/dispatch.rs index c18b9b77d8..b3b0ad26e5 100644 --- a/codex-rs/core/src/memories/startup/dispatch.rs +++ b/codex-rs/core/src/memories/startup/dispatch.rs @@ -1,6 +1,6 @@ use crate::codex::Session; use crate::config::Config; -use crate::memories::layout::memory_root; +use crate::memories::memory_root; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_protocol::user_input::UserInput; @@ -17,7 +17,7 @@ use super::super::prompts::build_consolidation_prompt; use super::super::storage::rebuild_raw_memories_file_from_memories; use super::super::storage::sync_rollout_summaries_from_memories; use super::super::storage::wipe_consolidation_outputs; -use super::watch::spawn_phase2_completion_task; +use super::phase2::spawn_phase2_completion_task; pub(super) async fn run_global_memory_consolidation( session: &Arc, diff --git a/codex-rs/core/src/memories/startup/extract.rs b/codex-rs/core/src/memories/startup/extract.rs index dd5f25854a..d4e9c23124 100644 --- a/codex-rs/core/src/memories/startup/extract.rs +++ b/codex-rs/core/src/memories/startup/extract.rs @@ -12,13 +12,13 @@ use futures::StreamExt; use tracing::warn; use super::StageOneRequestContext; +use crate::memories::StageOneOutput; use crate::memories::prompts::build_stage_one_input_message; use crate::memories::rollout::StageOneRolloutFilter; use crate::memories::rollout::serialize_filtered_rollout_response_items; use crate::memories::stage_one::RAW_MEMORY_PROMPT; use crate::memories::stage_one::parse_stage_one_output; use crate::memories::stage_one::stage_one_output_schema; -use crate::memories::types::StageOneOutput; use std::path::Path; pub(super) async fn extract_stage_one_output( diff --git a/codex-rs/core/src/memories/startup/mod.rs b/codex-rs/core/src/memories/startup/mod.rs index 985d4d2758..3608defa15 100644 --- a/codex-rs/core/src/memories/startup/mod.rs +++ b/codex-rs/core/src/memories/startup/mod.rs @@ -1,13 +1,12 @@ mod dispatch; mod extract; -mod watch; +mod phase2; use crate::codex::Session; use crate::codex::TurnContext; use crate::config::Config; use crate::error::Result as CodexResult; use crate::features::Feature; -use crate::memories::layout::migrate_legacy_user_memory_root_if_needed; use crate::rollout::INTERACTIVE_SESSION_SOURCES; use codex_otel::OtelManager; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; @@ -15,7 +14,6 @@ use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::protocol::SessionSource; use futures::StreamExt; -use serde_json::Value; use std::sync::Arc; use tracing::info; use tracing::warn; @@ -80,10 +78,6 @@ pub(super) async fn run_memories_startup_pipeline( session: &Arc, config: Arc, ) -> CodexResult<()> { - if let Err(err) = migrate_legacy_user_memory_root_if_needed(&config.codex_home).await { - warn!("failed migrating legacy shared memory root: {err}"); - } - let Some(state_db) = session.services.state_db.as_deref() else { warn!("state db unavailable for memories startup pipeline; skipping"); return Ok(()); @@ -91,11 +85,7 @@ pub(super) async fn run_memories_startup_pipeline( let allowed_sources = INTERACTIVE_SESSION_SOURCES .iter() - .map(|value| match serde_json::to_value(value) { - Ok(Value::String(s)) => s, - Ok(other) => other.to_string(), - Err(_) => String::new(), - }) + .map(ToString::to_string) .collect::>(); let claimed_candidates = match state_db @@ -186,7 +176,8 @@ pub(super) async fn run_memories_startup_pipeline( claimed_count, succeeded_count ); - let consolidation_job_count = run_consolidation_dispatch(session, config).await; + let consolidation_job_count = + usize::from(dispatch::run_global_memory_consolidation(session, config).await); info!( "memory consolidation dispatch complete: {} job(s) scheduled", consolidation_job_count @@ -194,7 +185,3 @@ pub(super) async fn run_memories_startup_pipeline( Ok(()) } - -async fn run_consolidation_dispatch(session: &Arc, config: Arc) -> usize { - usize::from(dispatch::run_global_memory_consolidation(session, config).await) -} diff --git a/codex-rs/core/src/memories/startup/watch.rs b/codex-rs/core/src/memories/startup/phase2.rs similarity index 100% rename from codex-rs/core/src/memories/startup/watch.rs rename to codex-rs/core/src/memories/startup/phase2.rs diff --git a/codex-rs/core/src/memories/storage.rs b/codex-rs/core/src/memories/storage.rs index 049130a5cf..aa14edf668 100644 --- a/codex-rs/core/src/memories/storage.rs +++ b/codex-rs/core/src/memories/storage.rs @@ -5,13 +5,12 @@ use std::path::Path; use tracing::warn; use super::MAX_RAW_MEMORIES_FOR_GLOBAL; +use super::MEMORY_REGISTRY_FILENAME; +use super::SKILLS_SUBDIR; +use super::ensure_layout; +use super::raw_memories_file; +use super::rollout_summaries_dir; use super::text::compact_whitespace; -use crate::memories::layout::LEGACY_CONSOLIDATED_FILENAME; -use crate::memories::layout::MEMORY_REGISTRY_FILENAME; -use crate::memories::layout::SKILLS_SUBDIR; -use crate::memories::layout::ensure_layout; -use crate::memories::layout::raw_memories_file; -use crate::memories::layout::rollout_summaries_dir; /// Rebuild `raw_memories.md` from DB-backed stage-1 outputs. pub(super) async fn rebuild_raw_memories_file_from_memories( @@ -49,16 +48,14 @@ pub(super) async fn sync_rollout_summaries_from_memories( /// /// Phase-1 artifacts (`rollout_summaries/` and `raw_memories.md`) are preserved. pub(super) async fn wipe_consolidation_outputs(root: &Path) -> std::io::Result<()> { - for file_name in [MEMORY_REGISTRY_FILENAME, LEGACY_CONSOLIDATED_FILENAME] { - let path = root.join(file_name); - if let Err(err) = tokio::fs::remove_file(&path).await - && err.kind() != std::io::ErrorKind::NotFound - { - warn!( - "failed removing consolidation file {}: {err}", - path.display() - ); - } + let path = root.join(MEMORY_REGISTRY_FILENAME); + if let Err(err) = tokio::fs::remove_file(&path).await + && err.kind() != std::io::ErrorKind::NotFound + { + warn!( + "failed removing consolidation file {}: {err}", + path.display() + ); } let skills_dir = root.join(SKILLS_SUBDIR); @@ -152,7 +149,7 @@ async fn write_rollout_summary_for_thread( .map_err(|err| std::io::Error::other(format!("format rollout summary: {err}")))?; writeln!(body) .map_err(|err| std::io::Error::other(format!("format rollout summary: {err}")))?; - body.push_str(&compact_whitespace(&memory.summary)); + body.push_str(&compact_whitespace(&memory.rollout_summary)); body.push('\n'); tokio::fs::write(path, body).await diff --git a/codex-rs/core/src/memories/tests.rs b/codex-rs/core/src/memories/tests.rs index 4de3fccba4..489951bae6 100644 --- a/codex-rs/core/src/memories/tests.rs +++ b/codex-rs/core/src/memories/tests.rs @@ -5,11 +5,10 @@ use super::stage_one::parse_stage_one_output; use super::storage::rebuild_raw_memories_file_from_memories; use super::storage::sync_rollout_summaries_from_memories; use super::storage::wipe_consolidation_outputs; -use crate::memories::layout::ensure_layout; -use crate::memories::layout::memory_root; -use crate::memories::layout::migrate_legacy_user_memory_root_if_needed; -use crate::memories::layout::raw_memories_file; -use crate::memories::layout::rollout_summaries_dir; +use crate::memories::ensure_layout; +use crate::memories::memory_root; +use crate::memories::raw_memories_file; +use crate::memories::rollout_summaries_dir; use chrono::TimeZone; use chrono::Utc; use codex_protocol::ThreadId; @@ -28,49 +27,18 @@ fn memory_root_uses_shared_global_path() { assert_eq!(memory_root(&codex_home), codex_home.join("memories")); } -#[tokio::test] -async fn migrate_legacy_user_memory_root_if_needed_copies_contents() { - let dir = tempdir().expect("tempdir"); - let codex_home = dir.path().join("codex"); - let legacy_root = codex_home.join("memories").join("user").join("memory"); - tokio::fs::create_dir_all(legacy_root.join("rollout_summaries")) - .await - .expect("create legacy rollout summaries dir"); - tokio::fs::write( - legacy_root.join("rollout_summaries").join("thread.md"), - "summary", - ) - .await - .expect("write legacy rollout summary"); - tokio::fs::write(legacy_root.join("raw_memories.md"), "raw") - .await - .expect("write legacy raw memories"); - - migrate_legacy_user_memory_root_if_needed(&codex_home) - .await - .expect("migrate legacy memory root"); - - let root = memory_root(&codex_home); - assert!(root.join("rollout_summaries").join("thread.md").is_file()); - assert!(root.join("raw_memories.md").is_file()); -} - #[test] fn parse_stage_one_output_accepts_fenced_json() { - let raw = "```json\n{\"raw_memory\":\"abc\",\"rollout_summary\":\"short\",\"rollout_slug\":\"slug\"}\n```"; + let raw = "```json\n{\"raw_memory\":\"abc\",\"rollout_summary\":\"short\"}\n```"; let parsed = parse_stage_one_output(raw).expect("parsed"); assert!(parsed.raw_memory.contains("abc")); assert_eq!(parsed.rollout_summary, "short"); - assert_eq!(parsed.rollout_slug, Some("slug".to_string())); } #[test] -fn parse_stage_one_output_accepts_legacy_keys() { +fn parse_stage_one_output_rejects_legacy_keys() { let raw = r#"{"rawMemory":"abc","summary":"short"}"#; - let parsed = parse_stage_one_output(raw).expect("parsed"); - assert!(parsed.raw_memory.contains("abc")); - assert_eq!(parsed.rollout_summary, "short"); - assert_eq!(parsed.rollout_slug, None); + assert!(parse_stage_one_output(raw).is_err()); } #[test] @@ -194,7 +162,7 @@ async fn sync_rollout_summaries_and_raw_memories_file_keeps_latest_memories_only thread_id: ThreadId::try_from(keep_id.clone()).expect("thread id"), source_updated_at: Utc.timestamp_opt(100, 0).single().expect("timestamp"), raw_memory: "raw memory".to_string(), - summary: "short summary".to_string(), + rollout_summary: "short summary".to_string(), generated_at: Utc.timestamp_opt(101, 0).single().expect("timestamp"), }]; @@ -216,13 +184,12 @@ async fn sync_rollout_summaries_and_raw_memories_file_keeps_latest_memories_only } #[tokio::test] -async fn wipe_consolidation_outputs_removes_registry_skills_and_legacy_file() { +async fn wipe_consolidation_outputs_removes_registry_and_skills() { let dir = tempdir().expect("tempdir"); let root = dir.path().join("memory"); ensure_layout(&root).await.expect("ensure layout"); let memory_registry = root.join("MEMORY.md"); - let legacy_consolidated = root.join("consolidated.md"); let skills_dir = root.join("skills").join("example"); tokio::fs::create_dir_all(&skills_dir) @@ -231,15 +198,11 @@ async fn wipe_consolidation_outputs_removes_registry_skills_and_legacy_file() { tokio::fs::write(&memory_registry, "memory") .await .expect("write memory registry"); - tokio::fs::write(&legacy_consolidated, "legacy") - .await - .expect("write legacy consolidated"); wipe_consolidation_outputs(&root) .await .expect("wipe consolidation outputs"); assert!(!memory_registry.exists()); - assert!(!legacy_consolidated.exists()); assert!(!root.join("skills").exists()); } diff --git a/codex-rs/core/src/memories/types.rs b/codex-rs/core/src/memories/types.rs deleted file mode 100644 index 3b68a1c72a..0000000000 --- a/codex-rs/core/src/memories/types.rs +++ /dev/null @@ -1,15 +0,0 @@ -use serde::Deserialize; - -/// Parsed stage-1 model output payload. -#[derive(Debug, Clone, Deserialize)] -pub(super) struct StageOneOutput { - /// Detailed markdown raw memory for a single rollout. - #[serde(rename = "raw_memory", alias = "rawMemory", alias = "traceMemory")] - pub(super) raw_memory: String, - /// Optional rollout slug from the model output. Accepted but ignored. - #[serde(default)] - pub(super) rollout_slug: Option, - /// Compact summary line used for routing and indexing. - #[serde(rename = "rollout_summary", alias = "summary")] - pub(super) rollout_summary: String, -} diff --git a/codex-rs/core/templates/memories/consolidation.md b/codex-rs/core/templates/memories/consolidation.md index 20cdbea44c..61db722294 100644 --- a/codex-rs/core/templates/memories/consolidation.md +++ b/codex-rs/core/templates/memories/consolidation.md @@ -19,7 +19,7 @@ Consolidation goals: 4. Deduplicate aggressively and remove generic advice. Expected outputs for this directory (create/update as needed): -- `MEMORY.md`: merged durable memory registry for this CWD. +- `MEMORY.md`: merged durable memory registry for this shared memory root. - `skills//...`: optional skill folders when there is clear reusable procedure value. Do not rewrite phase-1 artifacts except when adding explicit cross-references: diff --git a/codex-rs/core/templates/memories/stage_one_input.md b/codex-rs/core/templates/memories/stage_one_input.md index 259a1a7596..8acbffacf7 100644 --- a/codex-rs/core/templates/memories/stage_one_input.md +++ b/codex-rs/core/templates/memories/stage_one_input.md @@ -1,4 +1,4 @@ -Analyze this rollout and produce `raw_memory`, `rollout_summary`, and optional `rollout_slug` as JSON. +Analyze this rollout and produce `raw_memory` and `rollout_summary` as JSON. rollout_context: - rollout_path: {{ rollout_path }} diff --git a/codex-rs/core/templates/memories/stage_one_system.md b/codex-rs/core/templates/memories/stage_one_system.md index 054cda9f98..847a0feeaa 100644 --- a/codex-rs/core/templates/memories/stage_one_system.md +++ b/codex-rs/core/templates/memories/stage_one_system.md @@ -4,7 +4,6 @@ You are given one rollout and must produce exactly one JSON object. Return exactly one JSON object with this schema: - raw_memory: a detailed markdown raw memory for this rollout only. - rollout_summary: a concise summary suitable for shared memory aggregation. -- rollout_slug: optional stable slug for the rollout (accepted but currently ignored). Input contract: - The user message contains: diff --git a/codex-rs/state/migrations/0011_generic_jobs_and_stage1_outputs.sql b/codex-rs/state/migrations/0006_memories.sql similarity index 61% rename from codex-rs/state/migrations/0011_generic_jobs_and_stage1_outputs.sql rename to codex-rs/state/migrations/0006_memories.sql index 9095c8b8e5..e5e34307fc 100644 --- a/codex-rs/state/migrations/0011_generic_jobs_and_stage1_outputs.sql +++ b/codex-rs/state/migrations/0006_memories.sql @@ -1,22 +1,16 @@ -DROP TABLE IF EXISTS thread_memory; -DROP TABLE IF EXISTS memory_phase1_jobs; -DROP TABLE IF EXISTS memory_scope_dirty; -DROP TABLE IF EXISTS memory_phase2_jobs; -DROP TABLE IF EXISTS memory_consolidation_locks; - -CREATE TABLE IF NOT EXISTS stage1_outputs ( +CREATE TABLE stage1_outputs ( thread_id TEXT PRIMARY KEY, source_updated_at INTEGER NOT NULL, raw_memory TEXT NOT NULL, - summary TEXT NOT NULL, + rollout_summary TEXT NOT NULL, generated_at INTEGER NOT NULL, FOREIGN KEY(thread_id) REFERENCES threads(id) ON DELETE CASCADE ); -CREATE INDEX IF NOT EXISTS idx_stage1_outputs_source_updated_at +CREATE INDEX idx_stage1_outputs_source_updated_at ON stage1_outputs(source_updated_at DESC, thread_id DESC); -CREATE TABLE IF NOT EXISTS jobs ( +CREATE TABLE jobs ( kind TEXT NOT NULL, job_key TEXT NOT NULL, status TEXT NOT NULL, @@ -33,5 +27,5 @@ CREATE TABLE IF NOT EXISTS jobs ( PRIMARY KEY (kind, job_key) ); -CREATE INDEX IF NOT EXISTS idx_jobs_kind_status_retry_lease +CREATE INDEX idx_jobs_kind_status_retry_lease ON jobs(kind, status, retry_at, lease_until); diff --git a/codex-rs/state/migrations/0006_thread_memory.sql b/codex-rs/state/migrations/0006_thread_memory.sql deleted file mode 100644 index fe90ab6679..0000000000 --- a/codex-rs/state/migrations/0006_thread_memory.sql +++ /dev/null @@ -1,9 +0,0 @@ -CREATE TABLE thread_memory ( - thread_id TEXT PRIMARY KEY, - trace_summary TEXT NOT NULL, - memory_summary TEXT NOT NULL, - updated_at INTEGER NOT NULL, - FOREIGN KEY(thread_id) REFERENCES threads(id) ON DELETE CASCADE -); - -CREATE INDEX idx_thread_memory_updated_at ON thread_memory(updated_at DESC, thread_id DESC); diff --git a/codex-rs/state/migrations/0009_memory_consolidation_locks.sql b/codex-rs/state/migrations/0009_memory_consolidation_locks.sql deleted file mode 100644 index 793ab4dc3b..0000000000 --- a/codex-rs/state/migrations/0009_memory_consolidation_locks.sql +++ /dev/null @@ -1,8 +0,0 @@ -CREATE TABLE memory_consolidation_locks ( - cwd TEXT PRIMARY KEY, - working_thread_id TEXT NOT NULL, - updated_at INTEGER NOT NULL -); - -CREATE INDEX idx_memory_consolidation_locks_updated_at - ON memory_consolidation_locks(updated_at DESC); diff --git a/codex-rs/state/migrations/0010_memory_workflow_v2.sql b/codex-rs/state/migrations/0010_memory_workflow_v2.sql deleted file mode 100644 index 475b5f6e38..0000000000 --- a/codex-rs/state/migrations/0010_memory_workflow_v2.sql +++ /dev/null @@ -1,85 +0,0 @@ -DROP TABLE IF EXISTS thread_memory; -DROP TABLE IF EXISTS memory_consolidation_locks; -DROP TABLE IF EXISTS memory_phase1_jobs; -DROP TABLE IF EXISTS memory_scope_dirty; -DROP TABLE IF EXISTS memory_phase2_jobs; - -CREATE TABLE thread_memory ( - thread_id TEXT NOT NULL, - scope_kind TEXT NOT NULL, - scope_key TEXT NOT NULL, - raw_memory TEXT NOT NULL, - memory_summary TEXT NOT NULL, - updated_at INTEGER NOT NULL, - last_used_at INTEGER, - used_count INTEGER NOT NULL DEFAULT 0, - invalidated_at INTEGER, - invalid_reason TEXT, - PRIMARY KEY (thread_id, scope_kind, scope_key), - FOREIGN KEY(thread_id) REFERENCES threads(id) ON DELETE CASCADE -); - -CREATE INDEX idx_thread_memory_scope_last_used_at - ON thread_memory(scope_kind, scope_key, last_used_at DESC, thread_id DESC); -CREATE INDEX idx_thread_memory_scope_updated_at - ON thread_memory(scope_kind, scope_key, updated_at DESC, thread_id DESC); - -CREATE TABLE memory_phase1_jobs ( - thread_id TEXT NOT NULL, - scope_kind TEXT NOT NULL, - scope_key TEXT NOT NULL, - status TEXT NOT NULL, - owner_session_id TEXT, - started_at INTEGER, - finished_at INTEGER, - failure_reason TEXT, - source_updated_at INTEGER NOT NULL, - raw_memory_path TEXT, - summary_hash TEXT, - ownership_token TEXT, - PRIMARY KEY (thread_id, scope_kind, scope_key), - FOREIGN KEY(thread_id) REFERENCES threads(id) ON DELETE CASCADE -); - -CREATE INDEX idx_memory_phase1_jobs_status_started_at - ON memory_phase1_jobs(status, started_at DESC); -CREATE INDEX idx_memory_phase1_jobs_scope - ON memory_phase1_jobs(scope_kind, scope_key); - -CREATE TABLE memory_scope_dirty ( - scope_kind TEXT NOT NULL, - scope_key TEXT NOT NULL, - dirty INTEGER NOT NULL, - updated_at INTEGER NOT NULL, - PRIMARY KEY (scope_kind, scope_key) -); - -CREATE INDEX idx_memory_scope_dirty_dirty - ON memory_scope_dirty(dirty, updated_at DESC); - -CREATE TABLE memory_phase2_jobs ( - scope_kind TEXT NOT NULL, - scope_key TEXT NOT NULL, - status TEXT NOT NULL, - owner_session_id TEXT, - agent_thread_id TEXT, - started_at INTEGER, - last_heartbeat_at INTEGER, - finished_at INTEGER, - attempt INTEGER NOT NULL DEFAULT 0, - failure_reason TEXT, - ownership_token TEXT, - PRIMARY KEY (scope_kind, scope_key) -); - -CREATE INDEX idx_memory_phase2_jobs_status_heartbeat - ON memory_phase2_jobs(status, last_heartbeat_at DESC); - -CREATE TABLE memory_consolidation_locks ( - cwd TEXT PRIMARY KEY, - working_thread_id TEXT NOT NULL, - updated_at INTEGER NOT NULL -); - -CREATE INDEX idx_memory_consolidation_locks_updated_at - ON memory_consolidation_locks(updated_at DESC); diff --git a/codex-rs/state/src/lib.rs b/codex-rs/state/src/lib.rs index 5564c36078..eb405a8499 100644 --- a/codex-rs/state/src/lib.rs +++ b/codex-rs/state/src/lib.rs @@ -14,6 +14,7 @@ mod runtime; pub use model::LogEntry; pub use model::LogQuery; pub use model::LogRow; +pub use model::Phase2JobClaimOutcome; /// Preferred entrypoint: owns configuration and metrics. pub use runtime::StateRuntime; @@ -27,16 +28,15 @@ pub use model::BackfillStats; pub use model::BackfillStatus; pub use model::ExtractionOutcome; pub use model::SortKey; +pub use model::Stage1JobClaim; +pub use model::Stage1JobClaimOutcome; pub use model::Stage1Output; +pub use model::Stage1StartupClaimParams; pub use model::ThreadMetadata; pub use model::ThreadMetadataBuilder; pub use model::ThreadsPage; -pub use runtime::Phase2JobClaimOutcome; pub use runtime::STATE_DB_FILENAME; pub use runtime::STATE_DB_VERSION; -pub use runtime::Stage1JobClaim; -pub use runtime::Stage1JobClaimOutcome; -pub use runtime::Stage1StartupClaimParams; pub use runtime::state_db_filename; pub use runtime::state_db_path; diff --git a/codex-rs/state/src/model/memories.rs b/codex-rs/state/src/model/memories.rs new file mode 100644 index 0000000000..81a7354c54 --- /dev/null +++ b/codex-rs/state/src/model/memories.rs @@ -0,0 +1,105 @@ +use anyhow::Result; +use chrono::DateTime; +use chrono::Utc; +use codex_protocol::ThreadId; +use sqlx::Row; +use sqlx::sqlite::SqliteRow; + +use super::ThreadMetadata; + +/// Stored stage-1 memory extraction output for a single thread. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Stage1Output { + pub thread_id: ThreadId, + pub source_updated_at: DateTime, + pub raw_memory: String, + pub rollout_summary: String, + pub generated_at: DateTime, +} + +#[derive(Debug)] +pub(crate) struct Stage1OutputRow { + thread_id: String, + source_updated_at: i64, + raw_memory: String, + rollout_summary: String, + generated_at: i64, +} + +impl Stage1OutputRow { + pub(crate) fn try_from_row(row: &SqliteRow) -> Result { + Ok(Self { + thread_id: row.try_get("thread_id")?, + source_updated_at: row.try_get("source_updated_at")?, + raw_memory: row.try_get("raw_memory")?, + rollout_summary: row.try_get("rollout_summary")?, + generated_at: row.try_get("generated_at")?, + }) + } +} + +impl TryFrom for Stage1Output { + type Error = anyhow::Error; + + fn try_from(row: Stage1OutputRow) -> std::result::Result { + Ok(Self { + thread_id: ThreadId::try_from(row.thread_id)?, + source_updated_at: epoch_seconds_to_datetime(row.source_updated_at)?, + raw_memory: row.raw_memory, + rollout_summary: row.rollout_summary, + generated_at: epoch_seconds_to_datetime(row.generated_at)?, + }) + } +} + +fn epoch_seconds_to_datetime(secs: i64) -> Result> { + DateTime::::from_timestamp(secs, 0) + .ok_or_else(|| anyhow::anyhow!("invalid unix timestamp: {secs}")) +} + +/// Result of trying to claim a stage-1 memory extraction job. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Stage1JobClaimOutcome { + /// The caller owns the job and should continue with extraction. + Claimed { ownership_token: String }, + /// Existing output is already newer than or equal to the source rollout. + SkippedUpToDate, + /// Another worker currently owns a fresh lease for this job. + SkippedRunning, + /// The job is in backoff and should not be retried yet. + SkippedRetryBackoff, + /// The job has exhausted retries and should not be retried automatically. + SkippedRetryExhausted, +} + +/// Claimed stage-1 job with thread metadata. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Stage1JobClaim { + pub thread: ThreadMetadata, + pub ownership_token: String, +} + +#[derive(Debug, Clone, Copy)] +pub struct Stage1StartupClaimParams<'a> { + pub scan_limit: usize, + pub max_claimed: usize, + pub max_age_days: i64, + pub min_rollout_idle_hours: i64, + pub allowed_sources: &'a [String], + pub lease_seconds: i64, +} + +/// Result of trying to claim a phase-2 consolidation job. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Phase2JobClaimOutcome { + /// The caller owns the global lock and should spawn consolidation. + Claimed { + ownership_token: String, + /// Snapshot of `input_watermark` at claim time. + input_watermark: i64, + }, + /// The global job is not pending consolidation (or is already up to date). + SkippedNotDirty, + /// Another worker currently owns a fresh global consolidation lease. + SkippedRunning, +} diff --git a/codex-rs/state/src/model/mod.rs b/codex-rs/state/src/model/mod.rs index 57fc805965..2a0009d647 100644 --- a/codex-rs/state/src/model/mod.rs +++ b/codex-rs/state/src/model/mod.rs @@ -1,6 +1,6 @@ mod backfill_state; mod log; -mod stage1_output; +mod memories; mod thread_metadata; pub use backfill_state::BackfillState; @@ -8,7 +8,11 @@ pub use backfill_state::BackfillStatus; pub use log::LogEntry; pub use log::LogQuery; pub use log::LogRow; -pub use stage1_output::Stage1Output; +pub use memories::Phase2JobClaimOutcome; +pub use memories::Stage1JobClaim; +pub use memories::Stage1JobClaimOutcome; +pub use memories::Stage1Output; +pub use memories::Stage1StartupClaimParams; pub use thread_metadata::Anchor; pub use thread_metadata::BackfillStats; pub use thread_metadata::ExtractionOutcome; @@ -17,7 +21,7 @@ pub use thread_metadata::ThreadMetadata; pub use thread_metadata::ThreadMetadataBuilder; pub use thread_metadata::ThreadsPage; -pub(crate) use stage1_output::Stage1OutputRow; +pub(crate) use memories::Stage1OutputRow; pub(crate) use thread_metadata::ThreadRow; pub(crate) use thread_metadata::anchor_from_item; pub(crate) use thread_metadata::datetime_to_epoch_seconds; diff --git a/codex-rs/state/src/model/stage1_output.rs b/codex-rs/state/src/model/stage1_output.rs deleted file mode 100644 index e69c6db623..0000000000 --- a/codex-rs/state/src/model/stage1_output.rs +++ /dev/null @@ -1,56 +0,0 @@ -use anyhow::Result; -use chrono::DateTime; -use chrono::Utc; -use codex_protocol::ThreadId; -use sqlx::Row; -use sqlx::sqlite::SqliteRow; - -/// Stored stage-1 memory extraction output for a single thread. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Stage1Output { - pub thread_id: ThreadId, - pub source_updated_at: DateTime, - pub raw_memory: String, - pub summary: String, - pub generated_at: DateTime, -} - -#[derive(Debug)] -pub(crate) struct Stage1OutputRow { - thread_id: String, - source_updated_at: i64, - raw_memory: String, - summary: String, - generated_at: i64, -} - -impl Stage1OutputRow { - pub(crate) fn try_from_row(row: &SqliteRow) -> Result { - Ok(Self { - thread_id: row.try_get("thread_id")?, - source_updated_at: row.try_get("source_updated_at")?, - raw_memory: row.try_get("raw_memory")?, - summary: row.try_get("summary")?, - generated_at: row.try_get("generated_at")?, - }) - } -} - -impl TryFrom for Stage1Output { - type Error = anyhow::Error; - - fn try_from(row: Stage1OutputRow) -> std::result::Result { - Ok(Self { - thread_id: ThreadId::try_from(row.thread_id)?, - source_updated_at: epoch_seconds_to_datetime(row.source_updated_at)?, - raw_memory: row.raw_memory, - summary: row.summary, - generated_at: epoch_seconds_to_datetime(row.generated_at)?, - }) - } -} - -fn epoch_seconds_to_datetime(secs: i64) -> Result> { - DateTime::::from_timestamp(secs, 0) - .ok_or_else(|| anyhow::anyhow!("invalid unix timestamp: {secs}")) -} diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index 21f55d068d..1a7b5f04a2 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -41,8 +41,8 @@ pub const STATE_DB_VERSION: u32 = 4; const METRIC_DB_INIT: &str = "codex.db.init"; -mod memory; -// Memory-specific CRUD and phase job lifecycle methods live in `runtime/memory.rs`. +mod memories; +// Memory-specific CRUD and phase job lifecycle methods live in `runtime/memories.rs`. #[derive(Clone)] pub struct StateRuntime { @@ -51,53 +51,6 @@ pub struct StateRuntime { pool: Arc, } -/// Result of trying to claim a stage-1 memory extraction job. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Stage1JobClaimOutcome { - /// The caller owns the job and should continue with extraction. - Claimed { ownership_token: String }, - /// Existing output is already newer than or equal to the source rollout. - SkippedUpToDate, - /// Another worker currently owns a fresh lease for this job. - SkippedRunning, - /// The job is in backoff and should not be retried yet. - SkippedRetryBackoff, - /// The job has exhausted retries and should not be retried automatically. - SkippedRetryExhausted, -} - -/// Claimed stage-1 job with thread metadata. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Stage1JobClaim { - pub thread: ThreadMetadata, - pub ownership_token: String, -} - -#[derive(Debug, Clone, Copy)] -pub struct Stage1StartupClaimParams<'a> { - pub scan_limit: usize, - pub max_claimed: usize, - pub max_age_days: i64, - pub min_rollout_idle_hours: i64, - pub allowed_sources: &'a [String], - pub lease_seconds: i64, -} - -/// Result of trying to claim a phase-2 consolidation job. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Phase2JobClaimOutcome { - /// The caller owns the global lock and should spawn consolidation. - Claimed { - ownership_token: String, - /// Snapshot of `input_watermark` at claim time. - input_watermark: i64, - }, - /// The global job is not pending consolidation (or is already up to date). - SkippedNotDirty, - /// Another worker currently owns a fresh global consolidation lease. - SkippedRunning, -} - impl StateRuntime { /// Initialize the state runtime using the provided Codex home and default provider. /// @@ -914,14 +867,14 @@ fn push_thread_order_and_limit( #[cfg(test)] mod tests { - use super::Phase2JobClaimOutcome; use super::STATE_DB_FILENAME; use super::STATE_DB_VERSION; - use super::Stage1JobClaimOutcome; - use super::Stage1StartupClaimParams; use super::StateRuntime; use super::ThreadMetadata; use super::state_db_filename; + use crate::model::Phase2JobClaimOutcome; + use crate::model::Stage1JobClaimOutcome; + use crate::model::Stage1StartupClaimParams; use chrono::DateTime; use chrono::Duration; use chrono::Utc; @@ -1582,9 +1535,9 @@ WHERE kind = 'memory_stage1' .expect("list stage1 outputs for global"); assert_eq!(outputs.len(), 2); assert_eq!(outputs[0].thread_id, thread_id_b); - assert_eq!(outputs[0].summary, "summary b"); + assert_eq!(outputs[0].rollout_summary, "summary b"); assert_eq!(outputs[1].thread_id, thread_id_a); - assert_eq!(outputs[1].summary, "summary a"); + assert_eq!(outputs[1].rollout_summary, "summary a"); let _ = tokio::fs::remove_dir_all(codex_home).await; } diff --git a/codex-rs/state/src/runtime/memory.rs b/codex-rs/state/src/runtime/memories.rs similarity index 97% rename from codex-rs/state/src/runtime/memory.rs rename to codex-rs/state/src/runtime/memories.rs index 65ce9b08f7..fdca79daf3 100644 --- a/codex-rs/state/src/runtime/memory.rs +++ b/codex-rs/state/src/runtime/memories.rs @@ -1,6 +1,10 @@ use super::*; -use crate::Stage1Output; +use crate::model::Phase2JobClaimOutcome; +use crate::model::Stage1JobClaim; +use crate::model::Stage1JobClaimOutcome; +use crate::model::Stage1Output; use crate::model::Stage1OutputRow; +use crate::model::Stage1StartupClaimParams; use crate::model::ThreadRow; use chrono::Duration; use sqlx::Executor; @@ -117,7 +121,7 @@ FROM threads ) -> anyhow::Result> { let row = sqlx::query( r#" -SELECT thread_id, source_updated_at, raw_memory, summary, generated_at +SELECT thread_id, source_updated_at, raw_memory, rollout_summary, generated_at FROM stage1_outputs WHERE thread_id = ? "#, @@ -140,9 +144,8 @@ WHERE thread_id = ? let rows = sqlx::query( r#" -SELECT so.thread_id, so.source_updated_at, so.raw_memory, so.summary, so.generated_at +SELECT so.thread_id, so.source_updated_at, so.raw_memory, so.rollout_summary, so.generated_at FROM stage1_outputs AS so -JOIN threads AS t ON t.id = so.thread_id ORDER BY so.source_updated_at DESC, so.thread_id DESC LIMIT ? "#, @@ -329,7 +332,7 @@ WHERE kind = ? AND job_key = ? ownership_token: &str, source_updated_at: i64, raw_memory: &str, - summary: &str, + rollout_summary: &str, ) -> anyhow::Result { let now = Utc::now().timestamp(); let thread_id = thread_id.to_string(); @@ -367,13 +370,13 @@ INSERT INTO stage1_outputs ( thread_id, source_updated_at, raw_memory, - summary, + rollout_summary, generated_at ) VALUES (?, ?, ?, ?, ?) ON CONFLICT(thread_id) DO UPDATE SET source_updated_at = excluded.source_updated_at, raw_memory = excluded.raw_memory, - summary = excluded.summary, + rollout_summary = excluded.rollout_summary, generated_at = excluded.generated_at WHERE excluded.source_updated_at >= stage1_outputs.source_updated_at "#, @@ -381,7 +384,7 @@ WHERE excluded.source_updated_at >= stage1_outputs.source_updated_at .bind(thread_id.as_str()) .bind(source_updated_at) .bind(raw_memory) - .bind(summary) + .bind(rollout_summary) .bind(now) .execute(&mut *tx) .await?; From 674799d3567fa6ea9913df597757d0a0f153ac6c Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 11 Feb 2026 00:02:57 +0000 Subject: [PATCH 5/7] feat: mem v2 - PR6 (consolidation) (#11374) --- codex-rs/codex-api/src/endpoint/memories.rs | 118 +++++++ codex-rs/core/src/client.rs | 100 ++++++ .../core/src/memories/startup/dispatch.rs | 129 +++++++- codex-rs/core/src/memories/startup/mod.rs | 18 + codex-rs/core/src/memories/startup/phase2.rs | 311 +++++++++++++++--- codex-rs/state/src/runtime.rs | 184 +++++++++++ codex-rs/state/src/runtime/memories.rs | 206 ++++++------ 7 files changed, 910 insertions(+), 156 deletions(-) diff --git a/codex-rs/codex-api/src/endpoint/memories.rs b/codex-rs/codex-api/src/endpoint/memories.rs index 310ee37cb3..13eb4e6a0d 100644 --- a/codex-rs/codex-api/src/endpoint/memories.rs +++ b/codex-rs/codex-api/src/endpoint/memories.rs @@ -67,11 +67,22 @@ struct SummarizeResponse { #[cfg(test)] mod tests { use super::*; + use crate::common::RawMemory; + use crate::common::RawMemoryMetadata; + use crate::provider::RetryConfig; use async_trait::async_trait; use codex_client::Request; use codex_client::Response; use codex_client::StreamResponse; use codex_client::TransportError; + use http::HeaderMap; + use http::Method; + use http::StatusCode; + use pretty_assertions::assert_eq; + use serde_json::json; + use std::sync::Arc; + use std::sync::Mutex; + use std::time::Duration; #[derive(Clone, Default)] struct DummyTransport; @@ -96,6 +107,54 @@ mod tests { } } + #[derive(Clone)] + struct CapturingTransport { + last_request: Arc>>, + response_body: Arc>, + } + + impl CapturingTransport { + fn new(response_body: Vec) -> Self { + Self { + last_request: Arc::new(Mutex::new(None)), + response_body: Arc::new(response_body), + } + } + } + + #[async_trait] + impl HttpTransport for CapturingTransport { + async fn execute(&self, req: Request) -> Result { + *self.last_request.lock().expect("lock request store") = Some(req); + Ok(Response { + status: StatusCode::OK, + headers: HeaderMap::new(), + body: self.response_body.as_ref().clone().into(), + }) + } + + async fn stream(&self, _req: Request) -> Result { + Err(TransportError::Build("stream should not run".to_string())) + } + } + + fn provider(base_url: &str) -> Provider { + Provider { + name: "test".to_string(), + base_url: base_url.to_string(), + query_params: None, + headers: HeaderMap::new(), + retry: RetryConfig { + max_attempts: 1, + base_delay: Duration::from_millis(1), + retry_429: false, + retry_5xx: true, + retry_transport: true, + }, + stream_idle_timeout: Duration::from_secs(1), + } + } + #[test] fn path_is_memories_trace_summarize_for_wire_compatibility() { assert_eq!( @@ -103,4 +162,63 @@ mod tests { "memories/trace_summarize" ); } + + #[tokio::test] + async fn summarize_input_posts_expected_payload_and_parses_output() { + let transport = CapturingTransport::new( + serde_json::to_vec(&json!({ + "output": [ + { + "trace_summary": "raw summary", + "memory_summary": "memory summary" + } + ] + })) + .expect("serialize response"), + ); + let client = MemoriesClient::new( + transport.clone(), + provider("https://example.com/api/codex"), + DummyAuth, + ); + + let input = MemorySummarizeInput { + model: "gpt-test".to_string(), + raw_memories: vec![RawMemory { + id: "trace-1".to_string(), + metadata: RawMemoryMetadata { + source_path: "/tmp/trace.json".to_string(), + }, + items: vec![json!({"type": "message", "role": "user", "content": []})], + }], + reasoning: None, + }; + + let output = client + .summarize_input(&input, HeaderMap::new()) + .await + .expect("summarize input request should succeed"); + assert_eq!(output.len(), 1); + assert_eq!(output[0].raw_memory, "raw summary"); + assert_eq!(output[0].memory_summary, "memory summary"); + + let request = transport + .last_request + .lock() + .expect("lock request store") + .clone() + .expect("request should be captured"); + assert_eq!(request.method, Method::POST); + assert_eq!( + request.url, + "https://example.com/api/codex/memories/trace_summarize" + ); + let body = request.body.expect("request body should be present"); + assert_eq!(body["model"], "gpt-test"); + assert_eq!(body["traces"][0]["id"], "trace-1"); + assert_eq!( + body["traces"][0]["metadata"]["source_path"], + "/tmp/trace.json" + ); + } } diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 1a6e2e0c48..ee4bc6f427 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -1116,3 +1116,103 @@ impl WebsocketTelemetry for ApiTelemetry { self.otel_manager.record_websocket_event(result, duration); } } + +#[cfg(test)] +mod tests { + use super::ModelClient; + use codex_otel::OtelManager; + use codex_protocol::ThreadId; + use codex_protocol::openai_models::ModelInfo; + use codex_protocol::protocol::SessionSource; + use codex_protocol::protocol::SubAgentSource; + use pretty_assertions::assert_eq; + use serde_json::json; + + fn test_model_client(session_source: SessionSource) -> ModelClient { + let provider = crate::model_provider_info::create_oss_provider_with_base_url( + "https://example.com/v1", + crate::model_provider_info::WireApi::Responses, + ); + ModelClient::new( + None, + ThreadId::new(), + provider, + session_source, + None, + false, + false, + false, + false, + None, + ) + } + + fn test_model_info() -> ModelInfo { + serde_json::from_value(json!({ + "slug": "gpt-test", + "display_name": "gpt-test", + "description": "desc", + "default_reasoning_level": "medium", + "supported_reasoning_levels": [ + {"effort": "medium", "description": "medium"} + ], + "shell_type": "shell_command", + "visibility": "list", + "supported_in_api": true, + "priority": 1, + "upgrade": null, + "base_instructions": "base instructions", + "model_messages": null, + "supports_reasoning_summaries": false, + "support_verbosity": false, + "default_verbosity": null, + "apply_patch_tool_type": null, + "truncation_policy": {"mode": "bytes", "limit": 10000}, + "supports_parallel_tool_calls": false, + "context_window": 272000, + "auto_compact_token_limit": null, + "experimental_supported_tools": [] + })) + .expect("deserialize test model info") + } + + fn test_otel_manager() -> OtelManager { + OtelManager::new( + ThreadId::new(), + "gpt-test", + "gpt-test", + None, + None, + None, + "test-originator".to_string(), + false, + "test-terminal".to_string(), + SessionSource::Cli, + ) + } + + #[test] + fn build_subagent_headers_sets_other_subagent_label() { + let client = test_model_client(SessionSource::SubAgent(SubAgentSource::Other( + "memory_consolidation".to_string(), + ))); + let headers = client.build_subagent_headers(); + let value = headers + .get("x-openai-subagent") + .and_then(|value| value.to_str().ok()); + assert_eq!(value, Some("memory_consolidation")); + } + + #[tokio::test] + async fn summarize_memories_returns_empty_for_empty_input() { + let client = test_model_client(SessionSource::Cli); + let model_info = test_model_info(); + let otel_manager = test_otel_manager(); + + let output = client + .summarize_memories(Vec::new(), &model_info, None, &otel_manager) + .await + .expect("empty summarize request should succeed"); + assert_eq!(output.len(), 0); + } +} diff --git a/codex-rs/core/src/memories/startup/dispatch.rs b/codex-rs/core/src/memories/startup/dispatch.rs index b3b0ad26e5..ebc39bec98 100644 --- a/codex-rs/core/src/memories/startup/dispatch.rs +++ b/codex-rs/core/src/memories/startup/dispatch.rs @@ -169,6 +169,7 @@ mod tests { use super::run_global_memory_consolidation; use crate::CodexAuth; use crate::ThreadManager; + use crate::agent::control::AgentControl; use crate::codex::Session; use crate::codex::make_session_and_context; use crate::config::Config; @@ -280,6 +281,14 @@ mod tests { .await .expect("shutdown spawned threads"); } + + fn user_input_ops_count(&self) -> usize { + self.manager + .captured_ops() + .into_iter() + .filter(|(_, op)| matches!(op, Op::UserInput { .. })) + .count() + } } #[tokio::test] @@ -311,12 +320,7 @@ mod tests { .expect("claim while running"); assert_eq!(running_claim, Phase2JobClaimOutcome::SkippedRunning); - let user_input_ops = harness - .manager - .captured_ops() - .into_iter() - .filter(|(_, op)| matches!(op, Op::UserInput { .. })) - .count(); + let user_input_ops = harness.user_input_ops_count(); assert_eq!(user_input_ops, 1); harness.shutdown_threads().await; @@ -338,14 +342,115 @@ mod tests { "second dispatch should skip while the global lock is running" ); - let user_input_ops = harness - .manager - .captured_ops() - .into_iter() - .filter(|(_, op)| matches!(op, Op::UserInput { .. })) - .count(); + let user_input_ops = harness.user_input_ops_count(); assert_eq!(user_input_ops, 1); harness.shutdown_threads().await; } + + #[tokio::test] + async fn dispatch_with_dirty_job_and_no_stage1_outputs_skips_spawn_and_clears_dirty_flag() { + let harness = DispatchHarness::new().await; + harness + .state_db + .enqueue_global_consolidation(999) + .await + .expect("enqueue global consolidation"); + + let scheduled = + run_global_memory_consolidation(&harness.session, Arc::clone(&harness.config)).await; + assert!( + !scheduled, + "dispatch should not spawn when no stage-1 outputs are available" + ); + assert_eq!(harness.user_input_ops_count(), 0); + + let claim = harness + .state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim global job after empty dispatch"); + assert_eq!( + claim, + Phase2JobClaimOutcome::SkippedNotDirty, + "empty dispatch should finalize global job as up-to-date" + ); + + harness.shutdown_threads().await; + } + + #[tokio::test] + async fn dispatch_marks_job_for_retry_when_spawn_agent_fails() { + let codex_home = tempfile::tempdir().expect("create temp codex home"); + let mut config = test_config(); + config.codex_home = codex_home.path().to_path_buf(); + config.cwd = config.codex_home.clone(); + let config = Arc::new(config); + + let state_db = codex_state::StateRuntime::init( + config.codex_home.clone(), + config.model_provider_id.clone(), + None, + ) + .await + .expect("initialize state db"); + + let (mut session, _turn_context) = make_session_and_context().await; + session.services.state_db = Some(Arc::clone(&state_db)); + session.services.agent_control = AgentControl::default(); + let session = Arc::new(session); + + let thread_id = ThreadId::new(); + let mut metadata_builder = ThreadMetadataBuilder::new( + thread_id, + config.codex_home.join(format!("rollout-{thread_id}.jsonl")), + Utc::now(), + SessionSource::Cli, + ); + metadata_builder.cwd = config.cwd.clone(); + metadata_builder.model_provider = Some(config.model_provider_id.clone()); + let metadata = metadata_builder.build(&config.model_provider_id); + state_db + .upsert_thread(&metadata) + .await + .expect("upsert thread metadata"); + + let claim = state_db + .try_claim_stage1_job(thread_id, session.conversation_id, 100, 3_600, 64) + .await + .expect("claim stage-1 job"); + let ownership_token = match claim { + codex_state::Stage1JobClaimOutcome::Claimed { ownership_token } => ownership_token, + other => panic!("unexpected stage-1 claim outcome: {other:?}"), + }; + assert!( + state_db + .mark_stage1_job_succeeded( + thread_id, + &ownership_token, + 100, + "raw memory", + "rollout summary", + ) + .await + .expect("mark stage-1 success"), + "stage-1 success should enqueue global consolidation" + ); + + let scheduled = run_global_memory_consolidation(&session, Arc::clone(&config)).await; + assert!( + !scheduled, + "dispatch should return false when consolidation subagent cannot be spawned" + ); + + let retry_claim = state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim global job after spawn failure"); + assert_eq!( + retry_claim, + Phase2JobClaimOutcome::SkippedNotDirty, + "spawn failures should leave the job in retry backoff instead of running" + ); + } } diff --git a/codex-rs/core/src/memories/startup/mod.rs b/codex-rs/core/src/memories/startup/mod.rs index 3608defa15..2be4ce9a3f 100644 --- a/codex-rs/core/src/memories/startup/mod.rs +++ b/codex-rs/core/src/memories/startup/mod.rs @@ -185,3 +185,21 @@ pub(super) async fn run_memories_startup_pipeline( Ok(()) } + +#[cfg(test)] +mod tests { + use super::run_memories_startup_pipeline; + use crate::codex::make_session_and_context; + use crate::config::test_config; + use std::sync::Arc; + + #[tokio::test] + async fn startup_pipeline_is_noop_when_state_db_is_unavailable() { + let (session, _turn_context) = make_session_and_context().await; + let session = Arc::new(session); + let config = Arc::new(test_config()); + run_memories_startup_pipeline(&session, config) + .await + .expect("startup pipeline should skip cleanly without state db"); + } +} diff --git a/codex-rs/core/src/memories/startup/phase2.rs b/codex-rs/core/src/memories/startup/phase2.rs index 8450bab752..d0d214b747 100644 --- a/codex-rs/core/src/memories/startup/phase2.rs +++ b/codex-rs/core/src/memories/startup/phase2.rs @@ -2,7 +2,9 @@ use crate::agent::AgentStatus; use crate::agent::status::is_final as is_final_agent_status; use crate::codex::Session; use codex_protocol::ThreadId; +use std::sync::Arc; use std::time::Duration; +use tokio::sync::watch; use tracing::debug; use tracing::info; use tracing::warn; @@ -21,32 +23,50 @@ pub(super) fn spawn_phase2_completion_task( let agent_control = session.services.agent_control.clone(); tokio::spawn(async move { - let Some(state_db) = state_db.as_deref() else { + let Some(state_db) = state_db else { return; }; - let mut status_rx = match agent_control.subscribe_status(consolidation_agent_id).await { + let status_rx = match agent_control.subscribe_status(consolidation_agent_id).await { Ok(status_rx) => status_rx, Err(err) => { warn!( "failed to subscribe to global memory consolidation agent {consolidation_agent_id}: {err}" ); - let _ = state_db - .mark_global_phase2_job_failed( - &ownership_token, - "failed to subscribe to consolidation agent status", - PHASE_TWO_JOB_RETRY_DELAY_SECONDS, - ) - .await; + mark_phase2_failed_with_recovery( + state_db.as_ref(), + &ownership_token, + "failed to subscribe to consolidation agent status", + ) + .await; return; } }; + run_phase2_completion_task( + Arc::clone(&state_db), + ownership_token, + completion_watermark, + consolidation_agent_id, + status_rx, + ) + .await; + }); +} + +async fn run_phase2_completion_task( + state_db: Arc, + ownership_token: String, + completion_watermark: i64, + consolidation_agent_id: ThreadId, + mut status_rx: watch::Receiver, +) { + let final_status = { let mut heartbeat_interval = tokio::time::interval(Duration::from_secs(PHASE_TWO_JOB_HEARTBEAT_SECONDS)); heartbeat_interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); - let final_status = loop { + loop { let status = status_rx.borrow().clone(); if is_final_agent_status(&status) { break status; @@ -68,55 +88,84 @@ pub(super) fn spawn_phase2_completion_task( { Ok(true) => {} Ok(false) => { - debug!( - "memory phase-2 heartbeat lost global ownership; skipping finalization" + warn!( + "memory phase-2 heartbeat lost global ownership; finalizing as failure" + ); + break AgentStatus::Errored( + "lost global phase-2 ownership during heartbeat".to_string(), ); - return; } Err(err) => { warn!( "state db heartbeat_global_phase2_job failed during memories startup: {err}" ); - return; + break AgentStatus::Errored(format!( + "phase-2 heartbeat update failed: {err}" + )); } } } } - }; - - if is_phase2_success(&final_status) { - match state_db - .mark_global_phase2_job_succeeded(&ownership_token, completion_watermark) - .await - { - Ok(true) => {} - Ok(false) => { - debug!( - "memory phase-2 success finalization skipped after global ownership changed" - ); - } - Err(err) => { - warn!( - "state db mark_global_phase2_job_succeeded failed during memories startup: {err}" - ); - } - } - info!( - "memory phase-2 global consolidation agent finished: agent_id={consolidation_agent_id} final_status={final_status:?}" - ); - return; } + }; - let failure_reason = phase2_failure_reason(&final_status); + if is_phase2_success(&final_status) { match state_db - .mark_global_phase2_job_failed( - &ownership_token, - &failure_reason, + .mark_global_phase2_job_succeeded(&ownership_token, completion_watermark) + .await + { + Ok(true) => {} + Ok(false) => { + debug!( + "memory phase-2 success finalization skipped after global ownership changed" + ); + } + Err(err) => { + warn!( + "state db mark_global_phase2_job_succeeded failed during memories startup: {err}" + ); + } + } + info!( + "memory phase-2 global consolidation agent finished: agent_id={consolidation_agent_id} final_status={final_status:?}" + ); + return; + } + + let failure_reason = phase2_failure_reason(&final_status); + mark_phase2_failed_with_recovery(state_db.as_ref(), &ownership_token, &failure_reason).await; + warn!( + "memory phase-2 global consolidation agent finished with non-success status: agent_id={consolidation_agent_id} final_status={final_status:?}" + ); +} + +async fn mark_phase2_failed_with_recovery( + state_db: &codex_state::StateRuntime, + ownership_token: &str, + failure_reason: &str, +) { + match state_db + .mark_global_phase2_job_failed( + ownership_token, + failure_reason, + PHASE_TWO_JOB_RETRY_DELAY_SECONDS, + ) + .await + { + Ok(true) => {} + Ok(false) => match state_db + .mark_global_phase2_job_failed_if_unowned( + ownership_token, + failure_reason, PHASE_TWO_JOB_RETRY_DELAY_SECONDS, ) .await { - Ok(true) => {} + Ok(true) => { + debug!( + "memory phase-2 failure finalization applied fallback update for unowned running job" + ); + } Ok(false) => { debug!( "memory phase-2 failure finalization skipped after global ownership changed" @@ -124,14 +173,14 @@ pub(super) fn spawn_phase2_completion_task( } Err(err) => { warn!( - "state db mark_global_phase2_job_failed failed during memories startup: {err}" + "state db mark_global_phase2_job_failed_if_unowned failed during memories startup: {err}" ); } + }, + Err(err) => { + warn!("state db mark_global_phase2_job_failed failed during memories startup: {err}"); } - warn!( - "memory phase-2 global consolidation agent finished with non-success status: agent_id={consolidation_agent_id} final_status={final_status:?}" - ); - }); + } } fn is_phase2_success(final_status: &AgentStatus) -> bool { @@ -146,7 +195,12 @@ fn phase2_failure_reason(final_status: &AgentStatus) -> String { mod tests { use super::is_phase2_success; use super::phase2_failure_reason; + use super::run_phase2_completion_task; use crate::agent::AgentStatus; + use codex_protocol::ThreadId; + use codex_state::Phase2JobClaimOutcome; + use pretty_assertions::assert_eq; + use std::sync::Arc; #[test] fn phase2_success_only_for_completed_status() { @@ -164,4 +218,167 @@ mod tests { assert!(reason.contains("consolidation agent finished with status")); assert!(reason.contains("boom")); } + + #[tokio::test] + async fn phase2_completion_marks_succeeded_for_completed_status() { + let codex_home = tempfile::tempdir().expect("create temp codex home"); + let state_db = Arc::new( + codex_state::StateRuntime::init( + codex_home.path().to_path_buf(), + "test-provider".to_string(), + None, + ) + .await + .expect("initialize state runtime"), + ); + let owner = ThreadId::new(); + state_db + .enqueue_global_consolidation(123) + .await + .expect("enqueue global consolidation"); + let claim = state_db + .try_claim_global_phase2_job(owner, 3_600) + .await + .expect("claim global phase-2 job"); + let ownership_token = match claim { + Phase2JobClaimOutcome::Claimed { + ownership_token, .. + } => ownership_token, + other => panic!("unexpected phase-2 claim outcome: {other:?}"), + }; + + let (_status_tx, status_rx) = tokio::sync::watch::channel(AgentStatus::Completed(None)); + run_phase2_completion_task( + Arc::clone(&state_db), + ownership_token.clone(), + 123, + ThreadId::new(), + status_rx, + ) + .await; + + let up_to_date_claim = state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim up-to-date global job"); + assert_eq!(up_to_date_claim, Phase2JobClaimOutcome::SkippedNotDirty); + + state_db + .enqueue_global_consolidation(124) + .await + .expect("enqueue advanced consolidation watermark"); + let rerun_claim = state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim rerun global job"); + assert!( + matches!(rerun_claim, Phase2JobClaimOutcome::Claimed { .. }), + "advanced watermark should be claimable after success finalization" + ); + } + + #[tokio::test] + async fn phase2_completion_marks_failed_when_status_updates_are_lost() { + let codex_home = tempfile::tempdir().expect("create temp codex home"); + let state_db = Arc::new( + codex_state::StateRuntime::init( + codex_home.path().to_path_buf(), + "test-provider".to_string(), + None, + ) + .await + .expect("initialize state runtime"), + ); + state_db + .enqueue_global_consolidation(456) + .await + .expect("enqueue global consolidation"); + let claim = state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim global phase-2 job"); + let ownership_token = match claim { + Phase2JobClaimOutcome::Claimed { + ownership_token, .. + } => ownership_token, + other => panic!("unexpected phase-2 claim outcome: {other:?}"), + }; + + let (status_tx, status_rx) = tokio::sync::watch::channel(AgentStatus::Running); + drop(status_tx); + run_phase2_completion_task( + Arc::clone(&state_db), + ownership_token, + 456, + ThreadId::new(), + status_rx, + ) + .await; + + let claim = state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim after failure finalization"); + assert_eq!( + claim, + Phase2JobClaimOutcome::SkippedNotDirty, + "failure finalization should leave global job in retry-backoff, not running ownership" + ); + } + + #[tokio::test] + async fn phase2_completion_heartbeat_loss_does_not_steal_active_other_owner() { + let codex_home = tempfile::tempdir().expect("create temp codex home"); + let state_db = Arc::new( + codex_state::StateRuntime::init( + codex_home.path().to_path_buf(), + "test-provider".to_string(), + None, + ) + .await + .expect("initialize state runtime"), + ); + state_db + .enqueue_global_consolidation(789) + .await + .expect("enqueue global consolidation"); + let claim = state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim global phase-2 job"); + let claimed_token = match claim { + Phase2JobClaimOutcome::Claimed { + ownership_token, .. + } => ownership_token, + other => panic!("unexpected phase-2 claim outcome: {other:?}"), + }; + + let (_status_tx, status_rx) = tokio::sync::watch::channel(AgentStatus::Running); + run_phase2_completion_task( + Arc::clone(&state_db), + "non-owner-token".to_string(), + 789, + ThreadId::new(), + status_rx, + ) + .await; + + let claim = state_db + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim after heartbeat ownership loss"); + assert_eq!( + claim, + Phase2JobClaimOutcome::SkippedRunning, + "heartbeat ownership-loss handling should not steal a live owner lease" + ); + assert_eq!( + state_db + .mark_global_phase2_job_succeeded(claimed_token.as_str(), 789) + .await + .expect("mark original owner success"), + true, + "the original owner should still be able to finalize" + ); + } } diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index 1a7b5f04a2..bd97979a5c 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -885,6 +885,7 @@ mod tests { use sqlx::Row; use std::path::Path; use std::path::PathBuf; + use std::sync::Arc; use std::time::SystemTime; use std::time::UNIX_EPOCH; use uuid::Uuid; @@ -1125,6 +1126,133 @@ mod tests { let _ = tokio::fs::remove_dir_all(codex_home).await; } + #[tokio::test] + async fn stage1_concurrent_claim_for_same_thread_is_conflict_safe() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + let thread_id = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("thread id"); + runtime + .upsert_thread(&test_thread_metadata( + &codex_home, + thread_id, + codex_home.join("workspace"), + )) + .await + .expect("upsert thread"); + + let owner_a = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner id"); + let owner_b = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner id"); + let thread_id_a = thread_id; + let thread_id_b = thread_id; + let runtime_a = Arc::clone(&runtime); + let runtime_b = Arc::clone(&runtime); + let claim_with_retry = |runtime: Arc, + thread_id: ThreadId, + owner: ThreadId| async move { + for attempt in 0..5 { + match runtime + .try_claim_stage1_job(thread_id, owner, 100, 3_600, 64) + .await + { + Ok(outcome) => return outcome, + Err(err) if err.to_string().contains("database is locked") && attempt < 4 => { + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + Err(err) => panic!("claim stage1 should not fail: {err}"), + } + } + panic!("claim stage1 should have returned within retry budget") + }; + + let (claim_a, claim_b) = tokio::join!( + claim_with_retry(runtime_a, thread_id_a, owner_a), + claim_with_retry(runtime_b, thread_id_b, owner_b), + ); + + let claim_outcomes = vec![claim_a, claim_b]; + let claimed_count = claim_outcomes + .iter() + .filter(|outcome| matches!(outcome, Stage1JobClaimOutcome::Claimed { .. })) + .count(); + assert_eq!(claimed_count, 1); + assert!( + claim_outcomes.iter().all(|outcome| { + matches!( + outcome, + Stage1JobClaimOutcome::Claimed { .. } | Stage1JobClaimOutcome::SkippedRunning + ) + }), + "unexpected claim outcomes: {claim_outcomes:?}" + ); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + + #[tokio::test] + async fn stage1_concurrent_claims_respect_running_cap() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + let thread_a = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("thread id"); + let thread_b = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("thread id"); + runtime + .upsert_thread(&test_thread_metadata( + &codex_home, + thread_a, + codex_home.join("workspace-a"), + )) + .await + .expect("upsert thread a"); + runtime + .upsert_thread(&test_thread_metadata( + &codex_home, + thread_b, + codex_home.join("workspace-b"), + )) + .await + .expect("upsert thread b"); + + let owner_a = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner id"); + let owner_b = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner id"); + let runtime_a = Arc::clone(&runtime); + let runtime_b = Arc::clone(&runtime); + + let (claim_a, claim_b) = tokio::join!( + async move { + runtime_a + .try_claim_stage1_job(thread_a, owner_a, 100, 3_600, 1) + .await + .expect("claim stage1 thread a") + }, + async move { + runtime_b + .try_claim_stage1_job(thread_b, owner_b, 101, 3_600, 1) + .await + .expect("claim stage1 thread b") + }, + ); + + let claim_outcomes = vec![claim_a, claim_b]; + let claimed_count = claim_outcomes + .iter() + .filter(|outcome| matches!(outcome, Stage1JobClaimOutcome::Claimed { .. })) + .count(); + assert_eq!(claimed_count, 1); + assert!( + claim_outcomes + .iter() + .any(|outcome| { matches!(outcome, Stage1JobClaimOutcome::SkippedRunning) }), + "one concurrent claim should be throttled by running cap: {claim_outcomes:?}" + ); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + #[tokio::test] async fn claim_stage1_jobs_filters_by_age_idle_and_current_thread() { let codex_home = unique_temp_dir(); @@ -1717,6 +1845,62 @@ WHERE kind = 'memory_stage1' let _ = tokio::fs::remove_dir_all(codex_home).await; } + #[tokio::test] + async fn phase2_failure_fallback_updates_unowned_running_job() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + runtime + .enqueue_global_consolidation(400) + .await + .expect("enqueue global consolidation"); + + let owner = ThreadId::from_string(&Uuid::new_v4().to_string()).expect("owner"); + let claim = runtime + .try_claim_global_phase2_job(owner, 3_600) + .await + .expect("claim global consolidation"); + let ownership_token = match claim { + Phase2JobClaimOutcome::Claimed { + ownership_token, .. + } => ownership_token, + other => panic!("unexpected claim outcome: {other:?}"), + }; + + sqlx::query("UPDATE jobs SET ownership_token = NULL WHERE kind = ? AND job_key = ?") + .bind("memory_consolidate_global") + .bind("global") + .execute(runtime.pool.as_ref()) + .await + .expect("clear ownership token"); + + assert_eq!( + runtime + .mark_global_phase2_job_failed(ownership_token.as_str(), "lost", 3_600) + .await + .expect("mark phase2 failed with strict ownership"), + false, + "strict failure update should not match unowned running job" + ); + assert!( + runtime + .mark_global_phase2_job_failed_if_unowned(ownership_token.as_str(), "lost", 3_600) + .await + .expect("fallback failure update should match unowned running job"), + "fallback failure update should transition the unowned running job" + ); + + let claim = runtime + .try_claim_global_phase2_job(ThreadId::new(), 3_600) + .await + .expect("claim after fallback failure"); + assert_eq!(claim, Phase2JobClaimOutcome::SkippedNotDirty); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + fn test_thread_metadata( codex_home: &Path, thread_id: ThreadId, diff --git a/codex-rs/state/src/runtime/memories.rs b/codex-rs/state/src/runtime/memories.rs index fdca79daf3..e321622213 100644 --- a/codex-rs/state/src/runtime/memories.rs +++ b/codex-rs/state/src/runtime/memories.rs @@ -194,67 +194,8 @@ WHERE thread_id = ? } } - let existing_job = sqlx::query( + let rows_affected = sqlx::query( r#" -SELECT status, lease_until, retry_at, retry_remaining -FROM jobs -WHERE kind = ? AND job_key = ? - "#, - ) - .bind(JOB_KIND_MEMORY_STAGE1) - .bind(thread_id.as_str()) - .fetch_optional(&mut *tx) - .await?; - - let should_insert = if let Some(existing_job) = existing_job { - let status: String = existing_job.try_get("status")?; - let existing_lease_until: Option = existing_job.try_get("lease_until")?; - let retry_at: Option = existing_job.try_get("retry_at")?; - let retry_remaining: i64 = existing_job.try_get("retry_remaining")?; - - if retry_remaining <= 0 { - tx.commit().await?; - return Ok(Stage1JobClaimOutcome::SkippedRetryExhausted); - } - if retry_at.is_some_and(|retry_at| retry_at > now) { - tx.commit().await?; - return Ok(Stage1JobClaimOutcome::SkippedRetryBackoff); - } - if status == "running" - && existing_lease_until.is_some_and(|lease_until| lease_until > now) - { - tx.commit().await?; - return Ok(Stage1JobClaimOutcome::SkippedRunning); - } - - false - } else { - true - }; - - let fresh_running_jobs = sqlx::query( - r#" -SELECT COUNT(*) AS count -FROM jobs -WHERE kind = ? - AND status = 'running' - AND lease_until IS NOT NULL - AND lease_until > ? - "#, - ) - .bind(JOB_KIND_MEMORY_STAGE1) - .bind(now) - .fetch_one(&mut *tx) - .await? - .try_get::("count")?; - if fresh_running_jobs >= max_running_jobs { - tx.commit().await?; - return Ok(Stage1JobClaimOutcome::SkippedRunning); - } - - if should_insert { - sqlx::query( - r#" INSERT INTO jobs ( kind, job_key, @@ -269,61 +210,96 @@ INSERT INTO jobs ( last_error, input_watermark, last_success_watermark -) VALUES (?, ?, 'running', ?, ?, ?, NULL, ?, NULL, ?, NULL, ?, NULL) - "#, - ) - .bind(JOB_KIND_MEMORY_STAGE1) - .bind(thread_id.as_str()) - .bind(worker_id.as_str()) - .bind(ownership_token.as_str()) - .bind(now) - .bind(lease_until) - .bind(DEFAULT_RETRY_REMAINING) - .bind(source_updated_at) - .execute(&mut *tx) - .await?; - tx.commit().await?; - return Ok(Stage1JobClaimOutcome::Claimed { ownership_token }); - } - - let rows_affected = sqlx::query( - r#" -UPDATE jobs -SET +) +SELECT ?, ?, 'running', ?, ?, ?, NULL, ?, NULL, ?, NULL, ?, NULL +WHERE ( + SELECT COUNT(*) + FROM jobs + WHERE kind = ? + AND status = 'running' + AND lease_until IS NOT NULL + AND lease_until > ? +) < ? +ON CONFLICT(kind, job_key) DO UPDATE SET status = 'running', - worker_id = ?, - ownership_token = ?, - started_at = ?, + worker_id = excluded.worker_id, + ownership_token = excluded.ownership_token, + started_at = excluded.started_at, finished_at = NULL, - lease_until = ?, + lease_until = excluded.lease_until, retry_at = NULL, last_error = NULL, - input_watermark = ? -WHERE kind = ? AND job_key = ? - AND (status != 'running' OR lease_until IS NULL OR lease_until <= ?) - AND (retry_at IS NULL OR retry_at <= ?) - AND retry_remaining > 0 + input_watermark = excluded.input_watermark +WHERE + (jobs.status != 'running' OR jobs.lease_until IS NULL OR jobs.lease_until <= excluded.started_at) + AND (jobs.retry_at IS NULL OR jobs.retry_at <= excluded.started_at) + AND jobs.retry_remaining > 0 + AND ( + SELECT COUNT(*) + FROM jobs AS running_jobs + WHERE running_jobs.kind = excluded.kind + AND running_jobs.status = 'running' + AND running_jobs.lease_until IS NOT NULL + AND running_jobs.lease_until > excluded.started_at + AND running_jobs.job_key != excluded.job_key + ) < ? "#, ) + .bind(JOB_KIND_MEMORY_STAGE1) + .bind(thread_id.as_str()) .bind(worker_id.as_str()) .bind(ownership_token.as_str()) .bind(now) .bind(lease_until) + .bind(DEFAULT_RETRY_REMAINING) .bind(source_updated_at) .bind(JOB_KIND_MEMORY_STAGE1) - .bind(thread_id.as_str()) - .bind(now) .bind(now) + .bind(max_running_jobs) + .bind(max_running_jobs) .execute(&mut *tx) .await? .rows_affected(); - tx.commit().await?; - if rows_affected == 0 { - Ok(Stage1JobClaimOutcome::SkippedRunning) - } else { - Ok(Stage1JobClaimOutcome::Claimed { ownership_token }) + if rows_affected > 0 { + tx.commit().await?; + return Ok(Stage1JobClaimOutcome::Claimed { ownership_token }); } + + let existing_job = sqlx::query( + r#" +SELECT status, lease_until, retry_at, retry_remaining +FROM jobs +WHERE kind = ? AND job_key = ? + "#, + ) + .bind(JOB_KIND_MEMORY_STAGE1) + .bind(thread_id.as_str()) + .fetch_optional(&mut *tx) + .await?; + + tx.commit().await?; + + if let Some(existing_job) = existing_job { + let status: String = existing_job.try_get("status")?; + let existing_lease_until: Option = existing_job.try_get("lease_until")?; + let retry_at: Option = existing_job.try_get("retry_at")?; + let retry_remaining: i64 = existing_job.try_get("retry_remaining")?; + + if retry_remaining <= 0 { + return Ok(Stage1JobClaimOutcome::SkippedRetryExhausted); + } + if retry_at.is_some_and(|retry_at| retry_at > now) { + return Ok(Stage1JobClaimOutcome::SkippedRetryBackoff); + } + if status == "running" + && existing_lease_until.is_some_and(|lease_until| lease_until > now) + { + return Ok(Stage1JobClaimOutcome::SkippedRunning); + } + } + + Ok(Stage1JobClaimOutcome::SkippedRunning) } pub async fn mark_stage1_job_succeeded( @@ -627,6 +603,42 @@ WHERE kind = ? AND job_key = ? Ok(rows_affected > 0) } + + pub async fn mark_global_phase2_job_failed_if_unowned( + &self, + ownership_token: &str, + failure_reason: &str, + retry_delay_seconds: i64, + ) -> anyhow::Result { + let now = Utc::now().timestamp(); + let retry_at = now.saturating_add(retry_delay_seconds.max(0)); + let rows_affected = sqlx::query( + r#" +UPDATE jobs +SET + status = 'error', + finished_at = ?, + lease_until = NULL, + retry_at = ?, + retry_remaining = retry_remaining - 1, + last_error = ? +WHERE kind = ? AND job_key = ? + AND status = 'running' + AND (ownership_token = ? OR ownership_token IS NULL) + "#, + ) + .bind(now) + .bind(retry_at) + .bind(failure_reason) + .bind(JOB_KIND_MEMORY_CONSOLIDATE_GLOBAL) + .bind(MEMORY_CONSOLIDATION_JOB_KEY) + .bind(ownership_token) + .execute(self.pool.as_ref()) + .await? + .rows_affected(); + + Ok(rows_affected > 0) + } } async fn enqueue_global_consolidation_with_executor<'e, E>( From 87bbfc50a1b82d0670147a1e63350f9bcf9259a4 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 11 Feb 2026 00:24:20 +0000 Subject: [PATCH 6/7] feat: prevent double backfill (#11377) ## Summary Add a DB-backed lease to prevent duplicate `.sqlite` backfill workers from running concurrently. ### What changed - Added StateRuntime::try_claim_backfill(lease_seconds) that atomically claims backfill only when: - backfill is not complete, and - no fresh running worker currently owns it. - Updated backfill_sessions to use the claim API and exit early when another worker already holds the lease. - Added runtime tests covering: - singleton claim behavior, - stale lease takeover, - claim blocked after complete. - Set backfill lease to 900s in production and 1s in tests. ### Why This avoids duplicate backfill work and reduces backfill status churn under concurrent startup, while preserving current best-effort fallback behavior. --- codex-rs/core/src/rollout/metadata.rs | 76 +++++++++++++++++++++---- codex-rs/state/src/runtime.rs | 80 +++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 11 deletions(-) diff --git a/codex-rs/core/src/rollout/metadata.rs b/codex-rs/core/src/rollout/metadata.rs index dc87c74da4..5bfdc363ae 100644 --- a/codex-rs/core/src/rollout/metadata.rs +++ b/codex-rs/core/src/rollout/metadata.rs @@ -30,6 +30,10 @@ use tracing::warn; const ROLLOUT_PREFIX: &str = "rollout-"; const ROLLOUT_SUFFIX: &str = ".jsonl"; const BACKFILL_BATCH_SIZE: usize = 200; +#[cfg(not(test))] +const BACKFILL_LEASE_SECONDS: i64 = 900; +#[cfg(test)] +const BACKFILL_LEASE_SECONDS: i64 = 1; pub(crate) fn builder_from_session_meta( session_meta: &SessionMetaLine, @@ -133,7 +137,7 @@ pub(crate) async fn backfill_sessions( otel: Option<&OtelManager>, ) { let timer = otel.and_then(|otel| otel.start_timer(DB_METRIC_BACKFILL_DURATION_MS, &[]).ok()); - let mut backfill_state = match runtime.get_backfill_state().await { + let backfill_state = match runtime.get_backfill_state().await { Ok(state) => state, Err(err) => { warn!( @@ -149,20 +153,66 @@ pub(crate) async fn backfill_sessions( if backfill_state.status == BackfillStatus::Complete { return; } - if let Err(err) = runtime.mark_backfill_running().await { - warn!( - "failed to mark backfill running at {}: {err}", + let claimed = match runtime.try_claim_backfill(BACKFILL_LEASE_SECONDS).await { + Ok(claimed) => claimed, + Err(err) => { + warn!( + "failed to claim backfill worker at {}: {err}", + config.codex_home.display() + ); + if let Some(otel) = otel { + otel.counter( + DB_ERROR_METRIC, + 1, + &[("stage", "backfill_state_claim_running")], + ); + } + return; + } + }; + if !claimed { + info!( + "state db backfill already running at {}; skipping duplicate worker", config.codex_home.display() ); - if let Some(otel) = otel { - otel.counter( - DB_ERROR_METRIC, - 1, - &[("stage", "backfill_state_mark_running")], + return; + } + let mut backfill_state = match runtime.get_backfill_state().await { + Ok(state) => state, + Err(err) => { + warn!( + "failed to read claimed backfill state at {}: {err}", + config.codex_home.display() ); + if let Some(otel) = otel { + otel.counter( + DB_ERROR_METRIC, + 1, + &[("stage", "backfill_state_read_claimed")], + ); + } + BackfillState { + status: BackfillStatus::Running, + ..Default::default() + } + } + }; + if backfill_state.status != BackfillStatus::Running { + if let Err(err) = runtime.mark_backfill_running().await { + warn!( + "failed to mark backfill running at {}: {err}", + config.codex_home.display() + ); + if let Some(otel) = otel { + otel.counter( + DB_ERROR_METRIC, + 1, + &[("stage", "backfill_state_mark_running")], + ); + } + } else { + backfill_state.status = BackfillStatus::Running; } - } else { - backfill_state.status = BackfillStatus::Running; } let sessions_root = config.codex_home.join(rollout::SESSIONS_SUBDIR); @@ -550,6 +600,10 @@ mod tests { .checkpoint_backfill(first_watermark.as_str()) .await .expect("checkpoint first watermark"); + tokio::time::sleep(std::time::Duration::from_secs( + (BACKFILL_LEASE_SECONDS + 1) as u64, + )) + .await; let mut config = crate::config::test_config(); config.codex_home = codex_home.clone(); diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index bd97979a5c..541e02ad38 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -108,6 +108,34 @@ WHERE id = 1 crate::BackfillState::try_from_row(&row) } + /// Attempt to claim ownership of rollout metadata backfill. + /// + /// Returns `true` when this runtime claimed the backfill worker slot. + /// Returns `false` if backfill is already complete or currently owned by a + /// non-expired worker. + pub async fn try_claim_backfill(&self, lease_seconds: i64) -> anyhow::Result { + self.ensure_backfill_state_row().await?; + let now = Utc::now().timestamp(); + let lease_cutoff = now.saturating_sub(lease_seconds.max(0)); + let result = sqlx::query( + r#" +UPDATE backfill_state +SET status = ?, updated_at = ? +WHERE id = 1 + AND status != ? + AND (status != ? OR updated_at <= ?) + "#, + ) + .bind(crate::BackfillStatus::Running.as_str()) + .bind(now) + .bind(crate::BackfillStatus::Complete.as_str()) + .bind(crate::BackfillStatus::Running.as_str()) + .bind(lease_cutoff) + .execute(self.pool.as_ref()) + .await?; + Ok(result.rows_affected() == 1) + } + /// Mark rollout metadata backfill as running. pub async fn mark_backfill_running(&self) -> anyhow::Result<()> { self.ensure_backfill_state_row().await?; @@ -1028,6 +1056,58 @@ mod tests { let _ = tokio::fs::remove_dir_all(codex_home).await; } + #[tokio::test] + async fn backfill_claim_is_singleton_until_stale_and_blocked_when_complete() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + let claimed = runtime + .try_claim_backfill(3600) + .await + .expect("initial backfill claim"); + assert_eq!(claimed, true); + + let duplicate_claim = runtime + .try_claim_backfill(3600) + .await + .expect("duplicate backfill claim"); + assert_eq!(duplicate_claim, false); + + let stale_updated_at = Utc::now().timestamp().saturating_sub(10_000); + sqlx::query( + r#" +UPDATE backfill_state +SET status = ?, updated_at = ? +WHERE id = 1 + "#, + ) + .bind(crate::BackfillStatus::Running.as_str()) + .bind(stale_updated_at) + .execute(runtime.pool.as_ref()) + .await + .expect("force stale backfill lease"); + + let stale_claim = runtime + .try_claim_backfill(10) + .await + .expect("stale backfill claim"); + assert_eq!(stale_claim, true); + + runtime + .mark_backfill_complete(None) + .await + .expect("mark complete"); + let claim_after_complete = runtime + .try_claim_backfill(3600) + .await + .expect("claim after complete"); + assert_eq!(claim_after_complete, false); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + #[tokio::test] async fn stage1_claim_skips_when_up_to_date() { let codex_home = unique_temp_dir(); From 9fe4ec9567f5884134d1e10ac4bfe86cd2c89fce Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 10 Feb 2026 17:43:11 -0800 Subject: [PATCH 7/7] Make sandbox read access configurable with ReadOnlyAccess ## What This change introduces a new `ReadOnlyAccess` model and threads it through sandbox policy consumers so read access is explicit instead of implicit. - Added `ReadOnlyAccess` to protocol: - `Restricted { include_platform_defaults, readable_roots }` - `FullAccess` - Changed `SandboxPolicy` shape: - `ReadOnly` is now `ReadOnly { access: ReadOnlyAccess }` - `WorkspaceWrite` now carries `read_only_access: ReadOnlyAccess` - Kept existing behavior for now by defaulting to `ReadOnlyAccess::FullAccess` in constructors and current config/app-server mappings. - Added helper methods to compute effective readable roots (including optional platform defaults + cwd) and to detect full read access. - Updated seatbelt policy generation to honor restricted read roots by emitting scoped `(allow file-read* ...)` entries when full read access is not granted. - Updated Linux backends (`bwrap`, legacy landlock path) to fail closed with an explicit `UnsupportedOperation` when restricted read access is requested but not yet implemented there. - Updated Windows sandbox backends (standard, elevated, and runner paths) to fail closed in the same way for restricted read access. - Updated all call sites/tests/pattern matches for the new structured variants and regenerated app-server protocol schema/types. ## Why The previous `SandboxPolicy::ReadOnly` implied full-disk read access and left no way to express a narrower read surface. This refactor establishes the policy model needed to support user-configurable read restrictions in a follow-up without changing current runtime behavior. It also ensures we do not silently ignore future restricted-read policies on platform backends that do not support them yet. Failing closed keeps sandbox semantics predictable and avoids accidental over-permission. ## Compatibility and rollout notes - Existing behavior is preserved by default (`FullAccess`). - Existing config/app-server flows continue to serialize/deserialize cleanly. - New schema artifacts are included to keep generated protocol outputs in sync. ## Validation - `just fmt` - `just fix -p codex-protocol -p codex-core -p codex-linux-sandbox -p codex-windows-sandbox -p codex-app-server-protocol` - `cargo check -p codex-windows-sandbox` - Targeted crate/test runs were executed during development for protocol/core/ sandbox-related crates. --- .../schema/json/ClientRequest.json | 69 ++++++- .../schema/json/EventMsg.json | 69 ++++++- .../schema/json/ServerNotification.json | 69 ++++++- .../codex_app_server_protocol.schemas.json | 69 ++++++- .../json/v1/ExecOneOffCommandParams.json | 69 ++++++- .../json/v1/ForkConversationResponse.json | 69 ++++++- .../json/v1/ResumeConversationResponse.json | 69 ++++++- .../schema/json/v1/SendUserTurnParams.json | 69 ++++++- .../v1/SessionConfiguredNotification.json | 69 ++++++- .../schema/typescript/ReadOnlyAccess.ts | 19 ++ .../schema/typescript/SandboxPolicy.ts | 11 +- .../schema/typescript/index.ts | 1 + .../app-server-protocol/src/protocol/v2.rs | 9 +- .../suite/codex_message_processor_flow.rs | 1 + codex-rs/common/src/approval_presets.rs | 2 +- codex-rs/common/src/sandbox_summary.rs | 4 +- codex-rs/core/src/config/mod.rs | 35 ++-- .../src/config_loader/config_requirements.rs | 11 +- codex-rs/core/src/config_loader/tests.rs | 3 +- codex-rs/core/src/connectors.rs | 2 +- codex-rs/core/src/exec.rs | 8 +- codex-rs/core/src/exec_policy.rs | 30 ++-- codex-rs/core/src/landlock.rs | 4 +- codex-rs/core/src/mcp/mod.rs | 2 +- codex-rs/core/src/rollout/metadata.rs | 2 +- codex-rs/core/src/safety.rs | 4 +- codex-rs/core/src/seatbelt.rs | 53 +++++- codex-rs/core/src/tools/registry.rs | 2 +- codex-rs/core/src/tools/sandboxing.rs | 5 +- codex-rs/core/tests/suite/apply_patch_cli.rs | 2 + codex-rs/core/tests/suite/approvals.rs | 34 ++-- codex-rs/core/tests/suite/codex_delegate.rs | 4 +- codex-rs/core/tests/suite/model_switching.rs | 12 +- .../core/tests/suite/permissions_messages.rs | 1 + codex-rs/core/tests/suite/personality.rs | 28 +-- codex-rs/core/tests/suite/prompt_caching.rs | 2 + codex-rs/core/tests/suite/rmcp_client.rs | 12 +- codex-rs/core/tests/suite/seatbelt.rs | 8 +- codex-rs/core/tests/suite/tools.rs | 2 +- codex-rs/core/tests/suite/truncation.rs | 6 +- codex-rs/core/tests/suite/unified_exec.rs | 4 +- codex-rs/core/tests/suite/web_search.rs | 29 +-- codex-rs/exec-server/src/posix/mcp.rs | 2 +- codex-rs/exec-server/tests/common/lib.rs | 3 +- .../tests/event_processor_with_json_output.rs | 2 +- codex-rs/exec/tests/suite/sandbox.rs | 8 +- codex-rs/linux-sandbox/src/bwrap.rs | 7 + codex-rs/linux-sandbox/src/landlock.rs | 14 +- codex-rs/linux-sandbox/src/linux_run_main.rs | 6 +- codex-rs/mcp-server/src/outgoing_message.rs | 6 +- codex-rs/protocol/src/models.rs | 3 +- codex-rs/protocol/src/protocol.rs | 168 +++++++++++++++++- codex-rs/state/src/model/thread_metadata.rs | 2 +- codex-rs/state/src/runtime.rs | 2 +- codex-rs/tui/src/additional_dirs.rs | 6 +- codex-rs/tui/src/app.rs | 18 +- codex-rs/tui/src/chatwidget.rs | 23 +-- codex-rs/tui/src/chatwidget/tests.rs | 11 +- codex-rs/tui/src/debug_config.rs | 2 +- codex-rs/tui/src/status/card.rs | 2 +- codex-rs/tui/src/status/tests.rs | 1 + codex-rs/windows-sandbox-rs/src/audit.rs | 2 +- .../src/command_runner_win.rs | 9 +- .../windows-sandbox-rs/src/elevated_impl.rs | 9 +- codex-rs/windows-sandbox-rs/src/lib.rs | 11 +- codex-rs/windows-sandbox-rs/src/policy.rs | 7 +- 66 files changed, 1107 insertions(+), 190 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index de746e7a78..4a29e8d145 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1243,6 +1243,57 @@ ], "type": "string" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -2018,8 +2069,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -2078,6 +2137,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 5991401b84..650bd16295 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -3518,6 +3518,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4355,8 +4406,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4415,6 +4474,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 69c6c2cb9b..eff42fa4db 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -4544,6 +4544,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -5461,8 +5512,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -5521,6 +5580,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" 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 88fb5ddbdb..c7c0a46874 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 @@ -6603,6 +6603,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -7616,8 +7667,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -7676,6 +7735,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json b/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json index a325704be4..ec5b789352 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ExecOneOffCommandParams.json @@ -13,6 +13,57 @@ ], "type": "string" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "SandboxPolicy": { "description": "Determines execution restrictions for model shell commands.", "oneOf": [ @@ -34,8 +85,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -94,6 +153,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index 90949d56f2..28ffb82cc4 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -3518,6 +3518,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4355,8 +4406,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4415,6 +4474,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index b25d2dbed3..5f21d65ec5 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -3518,6 +3518,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4355,8 +4406,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4415,6 +4474,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json b/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json index d56ae933bd..b2fc248663 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json @@ -142,6 +142,57 @@ ], "type": "string" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -195,8 +246,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -255,6 +314,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index fa9e144c48..8f2e441f86 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -3518,6 +3518,57 @@ ], "type": "object" }, + "ReadOnlyAccess": { + "description": "Determines how read-only file access is granted inside a restricted sandbox.", + "oneOf": [ + { + "description": "Restrict reads to an explicit set of roots.\n\nWhen `include_platform_defaults` is `true`, platform defaults required for basic execution are included in addition to `readable_roots`.", + "properties": { + "include_platform_defaults": { + "default": true, + "description": "Include built-in platform read roots required for basic process execution.", + "type": "boolean" + }, + "readable_roots": { + "description": "Additional absolute roots that should be readable.", + "items": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "type": "array" + }, + "type": { + "enum": [ + "restricted" + ], + "title": "RestrictedReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "RestrictedReadOnlyAccess", + "type": "object" + }, + { + "description": "Allow unrestricted file reads.", + "properties": { + "type": { + "enum": [ + "full-access" + ], + "title": "FullAccessReadOnlyAccessType", + "type": "string" + } + }, + "required": [ + "type" + ], + "title": "FullAccessReadOnlyAccess", + "type": "object" + } + ] + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ @@ -4355,8 +4406,16 @@ "type": "object" }, { - "description": "Read-only access to the entire file-system.", + "description": "Read-only access configuration.", "properties": { + "access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "read-only" @@ -4415,6 +4474,14 @@ "description": "When set to `true`, outbound network access is allowed. `false` by default.", "type": "boolean" }, + "read_only_access": { + "allOf": [ + { + "$ref": "#/definitions/ReadOnlyAccess" + } + ], + "description": "Read access granted while running under this policy." + }, "type": { "enum": [ "workspace-write" diff --git a/codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts b/codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts new file mode 100644 index 0000000000..c01bdd37c6 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/ReadOnlyAccess.ts @@ -0,0 +1,19 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AbsolutePathBuf } from "./AbsolutePathBuf"; + +/** + * Determines how read-only file access is granted inside a restricted + * sandbox. + */ +export type ReadOnlyAccess = { "type": "restricted", +/** + * Include built-in platform read roots required for basic process + * execution. + */ +include_platform_defaults: boolean, +/** + * Additional absolute roots that should be readable. + */ +readable_roots?: Array, } | { "type": "full-access" }; diff --git a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts index 103a6863f4..743ad22229 100644 --- a/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts +++ b/codex-rs/app-server-protocol/schema/typescript/SandboxPolicy.ts @@ -3,11 +3,16 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "./AbsolutePathBuf"; import type { NetworkAccess } from "./NetworkAccess"; +import type { ReadOnlyAccess } from "./ReadOnlyAccess"; /** * Determines execution restrictions for model shell commands. */ -export type SandboxPolicy = { "type": "danger-full-access" } | { "type": "read-only" } | { "type": "external-sandbox", +export type SandboxPolicy = { "type": "danger-full-access" } | { "type": "read-only", +/** + * Read access granted while running under this policy. + */ +access?: ReadOnlyAccess, } | { "type": "external-sandbox", /** * Whether the external sandbox permits outbound network traffic. */ @@ -17,6 +22,10 @@ network_access: NetworkAccess, } | { "type": "workspace-write", * writable from within the sandbox. */ writable_roots?: Array, +/** + * Read access granted while running under this policy. + */ +read_only_access?: ReadOnlyAccess, /** * When set to `true`, outbound network access is allowed. `false` by * default. diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 895936de2b..03675b3e10 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -137,6 +137,7 @@ export type { Profile } from "./Profile"; export type { RateLimitSnapshot } from "./RateLimitSnapshot"; export type { RateLimitWindow } from "./RateLimitWindow"; export type { RawResponseItemEvent } from "./RawResponseItemEvent"; +export type { ReadOnlyAccess } from "./ReadOnlyAccess"; export type { ReasoningContentDeltaEvent } from "./ReasoningContentDeltaEvent"; export type { ReasoningEffort } from "./ReasoningEffort"; export type { ReasoningItem } from "./ReasoningItem"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 679dc4ab23..37004e8081 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -32,6 +32,7 @@ use codex_protocol::protocol::CreditsSnapshot as CoreCreditsSnapshot; use codex_protocol::protocol::NetworkAccess as CoreNetworkAccess; use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; +use codex_protocol::protocol::ReadOnlyAccess as CoreReadOnlyAccess; use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::SkillDependencies as CoreSkillDependencies; use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; @@ -680,7 +681,9 @@ impl SandboxPolicy { SandboxPolicy::DangerFullAccess => { codex_protocol::protocol::SandboxPolicy::DangerFullAccess } - SandboxPolicy::ReadOnly => codex_protocol::protocol::SandboxPolicy::ReadOnly, + SandboxPolicy::ReadOnly => codex_protocol::protocol::SandboxPolicy::ReadOnly { + access: CoreReadOnlyAccess::FullAccess, + }, SandboxPolicy::ExternalSandbox { network_access } => { codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -696,6 +699,7 @@ impl SandboxPolicy { exclude_slash_tmp, } => codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots.clone(), + read_only_access: CoreReadOnlyAccess::FullAccess, network_access: *network_access, exclude_tmpdir_env_var: *exclude_tmpdir_env_var, exclude_slash_tmp: *exclude_slash_tmp, @@ -710,7 +714,7 @@ impl From for SandboxPolicy { codex_protocol::protocol::SandboxPolicy::DangerFullAccess => { SandboxPolicy::DangerFullAccess } - codex_protocol::protocol::SandboxPolicy::ReadOnly => SandboxPolicy::ReadOnly, + codex_protocol::protocol::SandboxPolicy::ReadOnly { .. } => SandboxPolicy::ReadOnly, codex_protocol::protocol::SandboxPolicy::ExternalSandbox { network_access } => { SandboxPolicy::ExternalSandbox { network_access: match network_access { @@ -721,6 +725,7 @@ impl From for SandboxPolicy { } codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { writable_roots, + read_only_access: _, network_access, exclude_tmpdir_env_var, exclude_slash_tmp, diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 2debbda653..863a7bfc4f 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -444,6 +444,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<( approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::WorkspaceWrite { writable_roots: vec![first_cwd.try_into()?], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/common/src/approval_presets.rs b/codex-rs/common/src/approval_presets.rs index cec67d258d..9e66e39e08 100644 --- a/codex-rs/common/src/approval_presets.rs +++ b/codex-rs/common/src/approval_presets.rs @@ -26,7 +26,7 @@ pub fn builtin_approval_presets() -> Vec { label: "Read Only", description: "Codex can read files in the current workspace. Approval is required to edit files or access the internet.", approval: AskForApproval::OnRequest, - sandbox: SandboxPolicy::ReadOnly, + sandbox: SandboxPolicy::new_read_only_policy(), }, ApprovalPreset { id: "auto", diff --git a/codex-rs/common/src/sandbox_summary.rs b/codex-rs/common/src/sandbox_summary.rs index 45520b11a0..53851d86b7 100644 --- a/codex-rs/common/src/sandbox_summary.rs +++ b/codex-rs/common/src/sandbox_summary.rs @@ -4,7 +4,7 @@ use codex_core::protocol::SandboxPolicy; pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { match sandbox_policy { SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), - SandboxPolicy::ReadOnly => "read-only".to_string(), + SandboxPolicy::ReadOnly { .. } => "read-only".to_string(), SandboxPolicy::ExternalSandbox { network_access } => { let mut summary = "external-sandbox".to_string(); if matches!(network_access, NetworkAccess::Enabled) { @@ -17,6 +17,7 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { network_access, exclude_tmpdir_env_var, exclude_slash_tmp, + read_only_access: _, } => { let mut summary = "workspace-write".to_string(); @@ -71,6 +72,7 @@ mod tests { let writable_root = AbsolutePathBuf::try_from(root).unwrap(); let summary = summarize_sandbox_policy(&SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root.clone()], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index a923b0ef78..1407436d25 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -43,6 +43,7 @@ use crate::model_provider_info::built_in_model_providers; use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME; use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME; use crate::protocol::AskForApproval; +use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; use crate::windows_sandbox::WindowsSandboxLevelExt; use codex_app_server_protocol::Tools; @@ -1183,6 +1184,7 @@ impl ConfigToml { exclude_slash_tmp, }) => SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots.clone(), + read_only_access: ReadOnlyAccess::FullAccess, network_access: *network_access, exclude_tmpdir_env_var: *exclude_tmpdir_env_var, exclude_slash_tmp: *exclude_slash_tmp, @@ -2105,7 +2107,7 @@ network_access = true # This should be ignored. assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: false, } ); @@ -2139,7 +2141,7 @@ exclude_slash_tmp = true assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: true, } ); @@ -2149,6 +2151,7 @@ exclude_slash_tmp = true SandboxPolicyResolution { policy: SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root.clone()], + read_only_access: ReadOnlyAccess::FullAccess, network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -2189,7 +2192,7 @@ trust_level = "trusted" assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: true, } ); @@ -2199,6 +2202,7 @@ trust_level = "trusted" SandboxPolicyResolution { policy: SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root], + read_only_access: ReadOnlyAccess::FullAccess, network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -2371,7 +2375,7 @@ trust_level = "trusted" "expected workspace-write request to be downgraded on Windows" ); match config.sandbox_policy.get() { - &SandboxPolicy::ReadOnly => {} + SandboxPolicy::ReadOnly { .. } => {} other => panic!("expected read-only policy on Windows, got {other:?}"), } } else { @@ -2517,7 +2521,10 @@ trust_level = "trusted" #[test] fn web_search_mode_for_turn_uses_preference_for_read_only() { let web_search_mode = Constrained::allow_any(WebSearchMode::Cached); - let mode = resolve_web_search_mode_for_turn(&web_search_mode, &SandboxPolicy::ReadOnly); + let mode = resolve_web_search_mode_for_turn( + &web_search_mode, + &SandboxPolicy::new_read_only_policy(), + ); assert_eq!(mode, WebSearchMode::Cached); } @@ -2700,7 +2707,7 @@ profile = "project" if cfg!(target_os = "windows") { assert!(matches!( config.sandbox_policy.get(), - SandboxPolicy::ReadOnly + SandboxPolicy::ReadOnly { .. } )); assert!(config.forced_auto_mode_downgraded_on_windows); } else { @@ -4672,7 +4679,7 @@ trust_level = "untrusted" // Verify that untrusted projects get WorkspaceWrite (or ReadOnly on Windows due to downgrade) if cfg!(target_os = "windows") { assert!( - matches!(resolution.policy, SandboxPolicy::ReadOnly), + matches!(resolution.policy, SandboxPolicy::ReadOnly { .. }), "Expected ReadOnly on Windows, got {:?}", resolution.policy ); @@ -4768,7 +4775,7 @@ trust_level = "untrusted" assert_eq!( resolution, SandboxPolicyResolution { - policy: SandboxPolicy::ReadOnly, + policy: SandboxPolicy::new_read_only_policy(), forced_auto_mode_downgraded_on_windows: true, } ); @@ -4924,7 +4931,7 @@ mcp_oauth_callback_port = 5678 // Verify that untrusted projects still get WorkspaceWrite sandbox (or ReadOnly on Windows) if cfg!(target_os = "windows") { assert!( - matches!(config.sandbox_policy.get(), SandboxPolicy::ReadOnly), + matches!(config.sandbox_policy.get(), SandboxPolicy::ReadOnly { .. }), "Expected ReadOnly on Windows" ); } else { @@ -4958,7 +4965,10 @@ mcp_oauth_callback_port = 5678 .build() .await?; - assert_eq!(*config.sandbox_policy.get(), SandboxPolicy::ReadOnly); + assert_eq!( + *config.sandbox_policy.get(), + SandboxPolicy::new_read_only_policy() + ); Ok(()) } @@ -4992,7 +5002,10 @@ mcp_oauth_callback_port = 5678 )) .build() .await?; - assert_eq!(*config.sandbox_policy.get(), SandboxPolicy::ReadOnly); + assert_eq!( + *config.sandbox_policy.get(), + SandboxPolicy::new_read_only_policy() + ); Ok(()) } diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs index 6ba68afebe..7e5ca96d00 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -94,7 +94,7 @@ impl Default for ConfigRequirements { None, ), sandbox_policy: ConstrainedWithSource::new( - Constrained::allow_any(SandboxPolicy::ReadOnly), + Constrained::allow_any(SandboxPolicy::new_read_only_policy()), None, ), web_search_mode: ConstrainedWithSource::new( @@ -421,7 +421,7 @@ impl TryFrom for ConfigRequirements { // the other variants (WorkspaceWrite, ExternalSandbox) require // additional parameters. Ultimately, we should expand the config // format to allow specifying those parameters. - let default_sandbox_policy = SandboxPolicy::ReadOnly; + let default_sandbox_policy = SandboxPolicy::new_read_only_policy(); let sandbox_policy = match allowed_sandbox_modes { Some(Sourced { value: modes, @@ -439,7 +439,7 @@ impl TryFrom for ConfigRequirements { let requirement_source_for_error = requirement_source.clone(); let constrained = Constrained::new(default_sandbox_policy, move |candidate| { let mode = match candidate { - SandboxPolicy::ReadOnly => SandboxModeRequirement::ReadOnly, + SandboxPolicy::ReadOnly { .. } => SandboxModeRequirement::ReadOnly, SandboxPolicy::WorkspaceWrite { .. } => { SandboxModeRequirement::WorkspaceWrite } @@ -873,7 +873,7 @@ mod tests { assert!( requirements .sandbox_policy - .can_set(&SandboxPolicy::ReadOnly) + .can_set(&SandboxPolicy::new_read_only_policy()) .is_ok() ); @@ -892,7 +892,7 @@ mod tests { assert!( requirements .sandbox_policy - .can_set(&SandboxPolicy::ReadOnly) + .can_set(&SandboxPolicy::new_read_only_policy()) .is_ok() ); assert!( @@ -900,6 +900,7 @@ mod tests { .sandbox_policy .can_set(&SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 0a6f67d85b..a47546dac7 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -397,7 +397,7 @@ allowed_sandbox_modes = ["read-only"] ); assert_eq!( *state.requirements().sandbox_policy.get(), - SandboxPolicy::ReadOnly + SandboxPolicy::new_read_only_policy() ); assert!( state @@ -412,6 +412,7 @@ allowed_sandbox_modes = ["read-only"] .sandbox_policy .can_set(&SandboxPolicy::WorkspaceWrite { writable_roots: Vec::new(), + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index b451d21b57..1769446b66 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -80,7 +80,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options( let cancel_token = CancellationToken::new(); let sandbox_state = SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index e627ebec06..398a767ce0 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -1094,7 +1094,13 @@ mod tests { arg0: None, }; - let output = exec(params, SandboxType::None, &SandboxPolicy::ReadOnly, None).await?; + let output = exec( + params, + SandboxType::None, + &SandboxPolicy::new_read_only_policy(), + None, + ) + .await?; assert!(output.timed_out); let stdout = output.stdout.from_utf8_lossy().text; diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 514b4f0af5..77c4d5e7bb 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -290,7 +290,7 @@ pub fn render_decision_for_unmatched_command( // On Windows, ReadOnly sandbox is not a real sandbox, so special-case it // here. let runtime_sandbox_provides_safety = - cfg!(windows) && matches!(sandbox_policy, SandboxPolicy::ReadOnly); + cfg!(windows) && matches!(sandbox_policy, SandboxPolicy::ReadOnly { .. }); // If the command is flagged as dangerous or we have no sandbox protection, // we should never allow it to run without user approval. @@ -325,7 +325,7 @@ pub fn render_decision_for_unmatched_command( // command has not been flagged as dangerous. Decision::Allow } - SandboxPolicy::ReadOnly | SandboxPolicy::WorkspaceWrite { .. } => { + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => { // In restricted sandboxes (ReadOnly/WorkspaceWrite), do not prompt for // non‑escalated, non‑dangerous commands — let the sandbox enforce // restrictions (e.g., block network/write) without a user prompt. @@ -832,7 +832,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -859,7 +859,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -887,7 +887,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: Some(requested_prefix.clone()), }) @@ -1008,7 +1008,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1038,7 +1038,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::RequireEscalated, prefix_rule: Some(vec!["cargo".to_string(), "install".to_string()]), }) @@ -1149,7 +1149,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1206,7 +1206,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1244,7 +1244,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::UnlessTrusted, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1267,7 +1267,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1297,7 +1297,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: SandboxPermissions::UseDefault, prefix_rule: None, }) @@ -1386,7 +1386,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &sneaky_command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: permissions, prefix_rule: None, }) @@ -1409,7 +1409,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &dangerous_command, approval_policy: AskForApproval::OnRequest, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: permissions, prefix_rule: None, }) @@ -1428,7 +1428,7 @@ prefix_rule( .create_exec_approval_requirement_for_command(ExecApprovalRequest { command: &dangerous_command, approval_policy: AskForApproval::Never, - sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_policy: &SandboxPolicy::new_read_only_policy(), sandbox_permissions: permissions, prefix_rule: None, }) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 51a4661938..65b2a68073 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -112,7 +112,7 @@ mod tests { fn bwrap_flags_are_feature_gated() { let command = vec!["/bin/true".to_string()]; let cwd = Path::new("/tmp"); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let with_bwrap = create_linux_sandbox_command_args(command.clone(), &policy, cwd, true, false); @@ -132,7 +132,7 @@ mod tests { fn proxy_flag_is_included_when_requested() { let command = vec!["/bin/true".to_string()]; let cwd = Path::new("/tmp"); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let args = create_linux_sandbox_command_args(command, &policy, cwd, true, true); assert_eq!( diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index 8034ed13a2..0ed0d221d2 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -165,7 +165,7 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent // Use ReadOnly sandbox policy for MCP snapshot collection (safest default) let sandbox_state = SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), diff --git a/codex-rs/core/src/rollout/metadata.rs b/codex-rs/core/src/rollout/metadata.rs index 5bfdc363ae..7298877751 100644 --- a/codex-rs/core/src/rollout/metadata.rs +++ b/codex-rs/core/src/rollout/metadata.rs @@ -49,7 +49,7 @@ pub(crate) fn builder_from_session_meta( builder.model_provider = session_meta.meta.model_provider.clone(); builder.cwd = session_meta.meta.cwd.clone(); builder.cli_version = Some(session_meta.meta.cli_version.clone()); - builder.sandbox_policy = SandboxPolicy::ReadOnly; + builder.sandbox_policy = SandboxPolicy::new_read_only_policy(); builder.approval_mode = AskForApproval::OnRequest; if let Some(git) = session_meta.git.as_ref() { builder.git_sha = git.commit_hash.clone(); diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 47a12e029e..b0d359b7e1 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -108,7 +108,7 @@ fn is_write_patch_constrained_to_writable_paths( ) -> bool { // Early‑exit if there are no declared writable roots. let writable_roots = match sandbox_policy { - SandboxPolicy::ReadOnly => { + SandboxPolicy::ReadOnly { .. } => { return false; } SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { @@ -195,6 +195,7 @@ mod tests { // only `cwd` is writable by default. let policy_workspace_only = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -216,6 +217,7 @@ mod tests { // outside write should be permitted. let policy_with_parent = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from(parent).unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index e2061b52dc..acd4638b0d 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -237,10 +237,40 @@ pub(crate) fn create_seatbelt_command_args( } }; - let file_read_policy = if sandbox_policy.has_full_disk_read_access() { - "; allow read-only file operations\n(allow file-read*)" + let (file_read_policy, file_read_dir_params) = if sandbox_policy.has_full_disk_read_access() { + ( + "; allow read-only file operations\n(allow file-read*)".to_string(), + Vec::new(), + ) } else { - "" + let mut readable_roots_policies: Vec = Vec::new(); + let mut file_read_params = Vec::new(); + for (index, root) in sandbox_policy + .get_readable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .enumerate() + { + // Canonicalize to avoid mismatches like /var vs /private/var on macOS. + let canonical_root = root + .as_path() + .canonicalize() + .unwrap_or_else(|_| root.to_path_buf()); + let root_param = format!("READABLE_ROOT_{index}"); + file_read_params.push((root_param.clone(), canonical_root)); + readable_roots_policies.push(format!("(subpath (param \"{root_param}\"))")); + } + + if readable_roots_policies.is_empty() { + ("".to_string(), Vec::new()) + } else { + ( + format!( + "; allow read-only file operations\n(allow file-read*\n{}\n)", + readable_roots_policies.join(" ") + ), + file_read_params, + ) + } }; let proxy = proxy_policy_inputs(network); @@ -250,7 +280,12 @@ pub(crate) fn create_seatbelt_command_args( "{MACOS_SEATBELT_BASE_POLICY}\n{file_read_policy}\n{file_write_policy}\n{network_policy}" ); - let dir_params = [file_write_dir_params, macos_dir_params()].concat(); + let dir_params = [ + file_read_dir_params, + file_write_dir_params, + macos_dir_params(), + ] + .concat(); let mut seatbelt_args: Vec = vec!["-p".to_string(), full_policy]; let definition_args = dir_params @@ -317,7 +352,7 @@ mod tests { #[test] fn create_seatbelt_args_routes_network_through_proxy_ports() { let policy = dynamic_network_policy( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), false, &ProxyPolicyInputs { ports: vec![43128, 48081], @@ -351,7 +386,7 @@ mod tests { #[test] fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() { let policy = dynamic_network_policy( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), false, &ProxyPolicyInputs { ports: vec![43128], @@ -383,6 +418,7 @@ mod tests { let policy = dynamic_network_policy( &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -410,6 +446,7 @@ mod tests { let policy = dynamic_network_policy( &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -430,6 +467,7 @@ mod tests { let policy = dynamic_network_policy( &SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -479,6 +517,7 @@ mod tests { .into_iter() .map(|p| p.try_into().unwrap()) .collect(), + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -664,6 +703,7 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![worktree_root.try_into().expect("worktree_root is absolute")], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -748,6 +788,7 @@ mod tests { // `.codex` checks are done properly for cwd. let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 3dca006d35..12fcc7e844 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -252,7 +252,7 @@ fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &str) -> Stri fn sandbox_policy_tag(policy: &SandboxPolicy) -> &'static str { match policy { - SandboxPolicy::ReadOnly => "read-only", + SandboxPolicy::ReadOnly { .. } => "read-only", SandboxPolicy::WorkspaceWrite { .. } => "workspace-write", SandboxPolicy::DangerFullAccess => "danger-full-access", SandboxPolicy::ExternalSandbox { .. } => "external-sandbox", diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 56e54e62f1..3671760844 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -326,7 +326,10 @@ mod tests { #[test] fn restricted_sandbox_requires_exec_approval_on_request() { assert_eq!( - default_exec_approval_requirement(AskForApproval::OnRequest, &SandboxPolicy::ReadOnly), + default_exec_approval_requirement( + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy() + ), ExecApprovalRequirement::NeedsApproval { reason: None, proposed_execpolicy_amendment: None, diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index ccfa9fe654..112f242bc0 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -578,6 +578,7 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace( let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -634,6 +635,7 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace( let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index ccf8760c8d..3b5e115ef8 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -628,6 +628,7 @@ fn scenarios() -> Vec { let workspace_write = |network_access| SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -841,7 +842,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_requires_approval", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_request.txt"), content: "read-only-approval", @@ -861,7 +862,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_requires_approval_gpt_5_1_no_exit", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_request_5_1.txt"), content: "read-only-approval", @@ -881,7 +882,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "trusted_command_on_request_read_only_runs_without_prompt", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunCommand { command: "echo trusted-read-only", }, @@ -896,7 +897,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "trusted_command_on_request_read_only_runs_without_prompt_gpt_5_1_no_exit", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunCommand { command: "echo trusted-read-only", }, @@ -911,7 +912,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_blocks_network", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::FetchUrl { endpoint: "/ro/network-blocked", response_body: "should-not-see", @@ -925,7 +926,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_denied_blocks_execution", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_request_denied.txt"), content: "should-not-write", @@ -946,7 +947,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_failure_escalates_after_sandbox_error", approval_policy: OnFailure, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_failure.txt"), content: "read-only-on-failure", @@ -967,7 +968,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_failure_escalates_after_sandbox_error_gpt_5_1_no_exit", approval_policy: OnFailure, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_on_failure_5_1.txt"), content: "read-only-on-failure", @@ -987,7 +988,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_network_escalates_when_approved", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::FetchUrl { endpoint: "/ro/network-approved", response_body: "read-only-network-ok", @@ -1006,7 +1007,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_on_request_network_escalates_when_approved_gpt_5_1_no_exit", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::FetchUrl { endpoint: "/ro/network-approved", response_body: "read-only-network-ok", @@ -1178,7 +1179,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_unless_trusted_requires_approval", approval_policy: UnlessTrusted, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_unless_trusted.txt"), content: "read-only-unless-trusted", @@ -1198,7 +1199,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_unless_trusted_requires_approval_gpt_5_1_no_exit", approval_policy: UnlessTrusted, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_unless_trusted_5_1.txt"), content: "read-only-unless-trusted", @@ -1218,7 +1219,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "read_only_never_reports_sandbox_failure", approval_policy: Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::WriteFile { target: TargetPath::Workspace("ro_never.txt"), content: "read-only-never", @@ -1242,7 +1243,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "trusted_command_never_runs_without_prompt", approval_policy: Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunCommand { command: "echo trusted-never", }, @@ -1407,7 +1408,7 @@ fn scenarios() -> Vec { ScenarioSpec { name: "unified exec on request escalated requires approval", approval_policy: OnRequest, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), action: ActionKind::RunUnifiedExecCommand { command: "python3 -c 'print('\"'\"'escalated unified exec'\"'\"')'", justification: Some(DEFAULT_UNIFIED_EXEC_JUSTIFICATION), @@ -1574,6 +1575,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() let approval_policy = AskForApproval::OnRequest; let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -1687,7 +1689,7 @@ async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts() -> Result<()> { let server = start_mock_server().await; let approval_policy = AskForApproval::UnlessTrusted; - let sandbox_policy = SandboxPolicy::ReadOnly; + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let sandbox_policy_for_config = sandbox_policy.clone(); let mut builder = test_codex().with_config(move |config| { config.approval_policy = Constrained::allow_any(approval_policy); diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index ea759ba5f4..36fa8b4c41 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -63,7 +63,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { // routes ExecApprovalRequest via the parent. let mut builder = test_codex().with_model("gpt-5.1").with_config(|config| { config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); - config.sandbox_policy = Constrained::allow_any(SandboxPolicy::ReadOnly); + config.sandbox_policy = Constrained::allow_any(SandboxPolicy::new_read_only_policy()); }); let test = builder.build(&server).await.expect("build test codex"); @@ -144,7 +144,7 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() { let mut builder = test_codex().with_model("gpt-5.1").with_config(|config| { config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); // Use a restricted sandbox so patch approval is required - config.sandbox_policy = Constrained::allow_any(SandboxPolicy::ReadOnly); + config.sandbox_policy = Constrained::allow_any(SandboxPolicy::new_read_only_policy()); config.include_apply_patch_tool = true; }); let test = builder.build(&server).await.expect("build test codex"); diff --git a/codex-rs/core/tests/suite/model_switching.rs b/codex-rs/core/tests/suite/model_switching.rs index b76dcb0467..3f9ec19688 100644 --- a/codex-rs/core/tests/suite/model_switching.rs +++ b/codex-rs/core/tests/suite/model_switching.rs @@ -53,7 +53,7 @@ async fn model_change_appends_model_instructions_developer_message() -> Result<( final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -86,7 +86,7 @@ async fn model_change_appends_model_instructions_developer_message() -> Result<( final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: next_model.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -141,7 +141,7 @@ async fn model_and_personality_change_only_appends_model_instructions() -> Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -174,7 +174,7 @@ async fn model_and_personality_change_only_appends_model_instructions() -> Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: next_model.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -288,7 +288,7 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result< final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: image_model_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -307,7 +307,7 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result< final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: text_model_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index fc1f0fa0b6..5bb92114f6 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -457,6 +457,7 @@ async fn permissions_message_includes_writable_roots() -> Result<()> { let writable_root = AbsolutePathBuf::try_from(writable.path())?; let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable_root], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 4fdedd0798..97fccabc69 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -94,7 +94,7 @@ async fn user_turn_personality_none_does_not_add_update_message() -> anyhow::Res final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -141,7 +141,7 @@ async fn config_personality_some_sets_instructions_template() -> anyhow::Result< final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -195,7 +195,7 @@ async fn config_personality_none_sends_no_personality() -> anyhow::Result<()> { final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -255,7 +255,7 @@ async fn default_personality_is_pragmatic_without_config_toml() -> anyhow::Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -303,7 +303,7 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -337,7 +337,7 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -400,7 +400,7 @@ async fn user_turn_personality_same_value_does_not_add_update_message() -> anyho final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -434,7 +434,7 @@ async fn user_turn_personality_same_value_does_not_add_update_message() -> anyho final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -506,7 +506,7 @@ async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -540,7 +540,7 @@ async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()> final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: test.config.approval_policy.value(), - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: test.session_configured.model.clone(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -651,7 +651,7 @@ async fn ignores_remote_personality_if_remote_models_disabled() -> anyhow::Resul final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -767,7 +767,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -881,7 +881,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, @@ -915,7 +915,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: remote_slug.to_string(), effort: test.config.model_reasoning_effort, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index e1ef097e20..64a9b55f8b 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -375,6 +375,7 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an let writable = TempDir::new().unwrap(); let new_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![writable.path().try_into().unwrap()], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -618,6 +619,7 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Res let writable = TempDir::new().unwrap(); let new_policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from(writable.path()).unwrap()], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 82d4d13a6e..a8ba430ab9 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -126,7 +126,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -293,7 +293,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -490,7 +490,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: text_only_model_slug.to_string(), effort: None, summary: ReasoningSummary::Auto, @@ -602,7 +602,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -761,7 +761,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -952,7 +952,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/seatbelt.rs b/codex-rs/core/tests/suite/seatbelt.rs index 614e87367a..89674494a9 100644 --- a/codex-rs/core/tests/suite/seatbelt.rs +++ b/codex-rs/core/tests/suite/seatbelt.rs @@ -77,6 +77,7 @@ async fn if_parent_of_repo_is_writable_then_dot_git_folder_is_writable() { let test_scenario = create_test_scenario(&tmp); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_parent.as_path().try_into().unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -103,6 +104,7 @@ async fn if_git_repo_is_writable_root_then_dot_git_folder_is_read_only() { let test_scenario = create_test_scenario(&tmp); let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_root.as_path().try_into().unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -145,7 +147,7 @@ async fn danger_full_access_allows_all_writes() { async fn read_only_forbids_all_writes() { let tmp = TempDir::new().expect("should be able to create temp dir"); let test_scenario = create_test_scenario(&tmp); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); test_scenario .run_test( @@ -171,7 +173,7 @@ async fn openpty_works_under_seatbelt() { return; } - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let command_cwd = std::env::current_dir().expect("getcwd"); let sandbox_cwd = command_cwd.clone(); @@ -229,7 +231,7 @@ async fn java_home_finds_runtime_under_seatbelt() { return; } - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let command_cwd = std::env::current_dir().expect("getcwd"); let sandbox_cwd = command_cwd.clone(); diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 7efa8bb28e..a828071ea5 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -230,7 +230,7 @@ async fn sandbox_denied_shell_returns_original_output() -> Result<()> { fixture .submit_turn_with_policy( "run a command that should be denied by the read-only sandbox", - SandboxPolicy::ReadOnly, + SandboxPolicy::new_read_only_policy(), ) .await?; diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index b36fef9eac..9475a9c510 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -384,7 +384,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> fixture .submit_turn_with_policy( "call the rmcp echo tool with a very large message", - SandboxPolicy::ReadOnly, + SandboxPolicy::new_read_only_policy(), ) .await?; @@ -485,7 +485,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { final_output_json_schema: None, cwd: fixture.cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -742,7 +742,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { fixture .submit_turn_with_policy( "call the rmcp echo tool with a very large message", - SandboxPolicy::ReadOnly, + SandboxPolicy::new_read_only_policy(), ) .await?; diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index c7b21413c5..34c5bd9617 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -2540,7 +2540,7 @@ async fn unified_exec_runs_under_sandbox() -> Result<()> { cwd: cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, // Important! - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, @@ -2644,7 +2644,7 @@ async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> { final_output_json_schema: None, cwd: cwd.path().to_path_buf(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), model: session_model, effort: None, summary: ReasoningSummary::Auto, diff --git a/codex-rs/core/tests/suite/web_search.rs b/codex-rs/core/tests/suite/web_search.rs index 65e8aedbed..8909e01cf4 100644 --- a/codex-rs/core/tests/suite/web_search.rs +++ b/codex-rs/core/tests/suite/web_search.rs @@ -44,9 +44,12 @@ async fn web_search_mode_cached_sets_external_web_access_false() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello cached web search", SandboxPolicy::ReadOnly) - .await - .expect("submit turn"); + test.submit_turn_with_policy( + "hello cached web search", + SandboxPolicy::new_read_only_policy(), + ) + .await + .expect("submit turn"); let body = resp_mock.single_request().body_json(); let tool = find_web_search_tool(&body); @@ -82,9 +85,12 @@ async fn web_search_mode_takes_precedence_over_legacy_flags() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello cached+live flags", SandboxPolicy::ReadOnly) - .await - .expect("submit turn"); + test.submit_turn_with_policy( + "hello cached+live flags", + SandboxPolicy::new_read_only_policy(), + ) + .await + .expect("submit turn"); let body = resp_mock.single_request().body_json(); let tool = find_web_search_tool(&body); @@ -121,9 +127,12 @@ async fn web_search_mode_defaults_to_cached_when_features_disabled() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello default cached web search", SandboxPolicy::ReadOnly) - .await - .expect("submit turn"); + test.submit_turn_with_policy( + "hello default cached web search", + SandboxPolicy::new_read_only_policy(), + ) + .await + .expect("submit turn"); let body = resp_mock.single_request().body_json(); let tool = find_web_search_tool(&body); @@ -169,7 +178,7 @@ async fn web_search_mode_updates_between_turns_with_sandbox_policy() { .await .expect("create test Codex conversation"); - test.submit_turn_with_policy("hello cached", SandboxPolicy::ReadOnly) + test.submit_turn_with_policy("hello cached", SandboxPolicy::new_read_only_policy()) .await .expect("submit first turn"); test.submit_turn_with_policy("hello live", SandboxPolicy::DangerFullAccess) diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs index 4cfce9ff89..7e8d0ccdff 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/posix/mcp.rs @@ -123,7 +123,7 @@ impl ExecTool { .await .clone() .unwrap_or_else(|| SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe: None, sandbox_cwd: PathBuf::from(¶ms.workdir), use_linux_sandbox_bwrap: false, diff --git a/codex-rs/exec-server/tests/common/lib.rs b/codex-rs/exec-server/tests/common/lib.rs index a51a589adf..430eb89953 100644 --- a/codex-rs/exec-server/tests/common/lib.rs +++ b/codex-rs/exec-server/tests/common/lib.rs @@ -88,7 +88,7 @@ where S: Service + ClientHandler, { let sandbox_state = SandboxState { - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), codex_linux_sandbox_exe, sandbox_cwd: sandbox_cwd.as_ref().to_path_buf(), use_linux_sandbox_bwrap: false, @@ -110,6 +110,7 @@ where // Note that sandbox_cwd will already be included as a writable root // when the sandbox policy is expanded. writable_roots: vec![], + read_only_access: Default::default(), network_access: false, // Disable writes to temp dir because this is a test, so // writable_folder is likely also under /tmp and we want to be diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index f349e4835f..d5f7126952 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -92,7 +92,7 @@ fn session_configured_produces_thread_started_event() { model: "codex-mini-latest".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 45de9c2236..ce2e79eac2 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -69,7 +69,7 @@ async fn spawn_command_under_sandbox( async fn linux_sandbox_test_env() -> Option> { let command_cwd = std::env::current_dir().ok()?; let sandbox_cwd = command_cwd.clone(); - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); if can_apply_linux_sandbox_policy(&policy, &command_cwd, sandbox_cwd.as_path(), HashMap::new()) .await @@ -134,6 +134,7 @@ async fn python_multiprocessing_lock_works_under_sandbox() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots, + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -194,7 +195,7 @@ async fn python_getpwuid_works_under_sandbox() { return; } - let policy = SandboxPolicy::ReadOnly; + let policy = SandboxPolicy::new_read_only_policy(); let command_cwd = std::env::current_dir().expect("should be able to get current dir"); let sandbox_cwd = command_cwd.clone(); @@ -247,6 +248,7 @@ async fn sandbox_distinguishes_command_and_policy_cwds() { // is under a writable root. let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, @@ -387,7 +389,7 @@ fn unix_sock_body() { async fn allow_unix_socketpair_recvfrom() { run_code_under_sandbox( "allow_unix_socketpair_recvfrom", - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), || async { unix_sock_body() }, ) .await diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 1a835fd5e0..7ff7acf5ef 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -141,6 +141,13 @@ fn create_bwrap_flags( /// 4. `--dev-bind /dev/null /dev/null` preserves the common sink even under a /// read-only root. fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result> { + if !sandbox_policy.has_full_disk_read_access() { + return Err(CodexErr::UnsupportedOperation( + "Restricted read-only access is not yet supported by the Linux bubblewrap backend." + .to_string(), + )); + } + let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); ensure_mount_targets_exist(&writable_roots)?; diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index d86fafd297..8d568dba0a 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -62,6 +62,13 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( } if apply_landlock_fs && !sandbox_policy.has_full_disk_write_access() { + if !sandbox_policy.has_full_disk_read_access() { + return Err(CodexErr::UnsupportedOperation( + "Restricted read-only access is not supported by the legacy Linux Landlock filesystem backend." + .to_string(), + )); + } + let writable_roots = sandbox_policy .get_writable_roots_with_cwd(cwd) .into_iter() @@ -70,9 +77,6 @@ pub(crate) fn apply_sandbox_policy_to_current_thread( install_filesystem_landlock_rules_on_current_thread(writable_roots)?; } - // TODO(ragona): Add appropriate restrictions if - // `sandbox_policy.has_full_disk_read_access()` is `false`. - Ok(()) } @@ -222,11 +226,11 @@ mod tests { #[test] fn restricted_network_policy_always_installs_seccomp() { assert!(should_install_network_seccomp( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), false )); assert!(should_install_network_seccomp( - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), true )); } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 2978efae2a..1df9a54438 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -391,7 +391,7 @@ mod tests { fn inserts_bwrap_argv0_before_command_separator() { let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), Path::new("/"), BwrapOptions { mount_proc: true, @@ -425,7 +425,7 @@ mod tests { fn inserts_unshare_net_when_network_isolation_requested() { let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), Path::new("/"), BwrapOptions { mount_proc: true, @@ -439,7 +439,7 @@ mod tests { fn inserts_unshare_net_when_proxy_only_network_mode_requested() { let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::ReadOnly, + &SandboxPolicy::new_read_only_policy(), Path::new("/"), BwrapOptions { mount_proc: true, diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 1bf98534b0..3f83e1990e 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -301,7 +301,7 @@ mod tests { model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, @@ -343,7 +343,7 @@ mod tests { model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, @@ -409,7 +409,7 @@ mod tests { model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffort::default()), history_log_id: 1, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 7f33199702..b5ce0f2b2c 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -312,7 +312,7 @@ impl DeveloperInstructions { let (sandbox_mode, writable_roots) = match sandbox_policy { SandboxPolicy::DangerFullAccess => (SandboxMode::DangerFullAccess, None), - SandboxPolicy::ReadOnly => (SandboxMode::ReadOnly, None), + SandboxPolicy::ReadOnly { .. } => (SandboxMode::ReadOnly, None), SandboxPolicy::ExternalSandbox { .. } => (SandboxMode::DangerFullAccess, None), SandboxPolicy::WorkspaceWrite { .. } => { let roots = sandbox_policy.get_writable_roots_with_cwd(cwd); @@ -1223,6 +1223,7 @@ mod tests { fn builds_permissions_from_policy() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: Default::default(), network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2368a39ba3..16f896435b 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -4,6 +4,7 @@ //! between user and agent. use std::collections::HashMap; +use std::collections::HashSet; use std::ffi::OsStr; use std::fmt; use std::path::Path; @@ -382,6 +383,107 @@ impl NetworkAccess { } } +fn default_include_platform_defaults() -> bool { + true +} + +/// Determines how read-only file access is granted inside a restricted +/// sandbox. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS)] +#[strum(serialize_all = "kebab-case")] +#[serde(tag = "type", rename_all = "kebab-case")] +#[ts(tag = "type")] +pub enum ReadOnlyAccess { + /// Restrict reads to an explicit set of roots. + /// + /// When `include_platform_defaults` is `true`, platform defaults required + /// for basic execution are included in addition to `readable_roots`. + Restricted { + /// Include built-in platform read roots required for basic process + /// execution. + #[serde(default = "default_include_platform_defaults")] + include_platform_defaults: bool, + /// Additional absolute roots that should be readable. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + readable_roots: Vec, + }, + + /// Allow unrestricted file reads. + #[default] + FullAccess, +} + +impl ReadOnlyAccess { + pub fn has_full_disk_read_access(&self) -> bool { + matches!(self, ReadOnlyAccess::FullAccess) + } + + /// Returns the readable roots for restricted read access. + /// + /// For [`ReadOnlyAccess::FullAccess`], returns an empty list because + /// callers should grant blanket read access instead. + pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec { + let mut roots: Vec = match self { + ReadOnlyAccess::FullAccess => return Vec::new(), + ReadOnlyAccess::Restricted { + include_platform_defaults, + readable_roots, + } => { + let mut roots = readable_roots.clone(); + if *include_platform_defaults { + #[cfg(target_os = "macos")] + for platform_path in [ + "/bin", "/dev", "/etc", "/Library", "/private", "/sbin", "/System", "/tmp", + "/usr", + ] { + #[allow(clippy::expect_used)] + roots.push( + AbsolutePathBuf::from_absolute_path(platform_path) + .expect("platform defaults should be absolute"), + ); + } + + #[cfg(target_os = "linux")] + for platform_path in ["/bin", "/dev", "/etc", "/lib", "/lib64", "/tmp", "/usr"] + { + #[allow(clippy::expect_used)] + roots.push( + AbsolutePathBuf::from_absolute_path(platform_path) + .expect("platform defaults should be absolute"), + ); + } + + #[cfg(target_os = "windows")] + for platform_path in [ + r"C:\Windows", + r"C:\Program Files", + r"C:\Program Files (x86)", + r"C:\ProgramData", + ] { + #[allow(clippy::expect_used)] + roots.push( + AbsolutePathBuf::from_absolute_path(platform_path) + .expect("platform defaults should be absolute"), + ); + } + + match AbsolutePathBuf::from_absolute_path(cwd) { + Ok(cwd_root) => roots.push(cwd_root), + Err(err) => { + error!("Ignoring invalid cwd {cwd:?} for sandbox readable root: {err}"); + } + } + } + roots + } + }; + + let mut seen = HashSet::new(); + roots.retain(|root| seen.insert(root.to_path_buf())); + roots + } +} + /// Determines execution restrictions for model shell commands. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)] #[strum(serialize_all = "kebab-case")] @@ -391,9 +493,16 @@ pub enum SandboxPolicy { #[serde(rename = "danger-full-access")] DangerFullAccess, - /// Read-only access to the entire file-system. + /// Read-only access configuration. #[serde(rename = "read-only")] - ReadOnly, + ReadOnly { + /// Read access granted while running under this policy. + #[serde( + default, + skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access" + )] + access: ReadOnlyAccess, + }, /// Indicates the process is already in an external sandbox. Allows full /// disk access while honoring the provided network setting. @@ -413,6 +522,13 @@ pub enum SandboxPolicy { #[serde(default, skip_serializing_if = "Vec::is_empty")] writable_roots: Vec, + /// Read access granted while running under this policy. + #[serde( + default, + skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access" + )] + read_only_access: ReadOnlyAccess, + /// When set to `true`, outbound network access is allowed. `false` by /// default. #[serde(default)] @@ -473,7 +589,9 @@ impl FromStr for SandboxPolicy { impl SandboxPolicy { /// Returns a policy with read-only disk access and no network. pub fn new_read_only_policy() -> Self { - SandboxPolicy::ReadOnly + SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + } } /// Returns a policy that can read the entire disk, but can only write to @@ -482,22 +600,29 @@ impl SandboxPolicy { pub fn new_workspace_write_policy() -> Self { SandboxPolicy::WorkspaceWrite { writable_roots: vec![], + read_only_access: ReadOnlyAccess::FullAccess, network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } } - /// Always returns `true`; restricting read access is not supported. pub fn has_full_disk_read_access(&self) -> bool { - true + match self { + SandboxPolicy::DangerFullAccess => true, + SandboxPolicy::ExternalSandbox { .. } => true, + SandboxPolicy::ReadOnly { access } => access.has_full_disk_read_access(), + SandboxPolicy::WorkspaceWrite { + read_only_access, .. + } => read_only_access.has_full_disk_read_access(), + } } pub fn has_full_disk_write_access(&self) -> bool { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { .. } => true, - SandboxPolicy::ReadOnly => false, + SandboxPolicy::ReadOnly { .. } => false, SandboxPolicy::WorkspaceWrite { .. } => false, } } @@ -506,11 +631,35 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => true, SandboxPolicy::ExternalSandbox { network_access } => network_access.is_enabled(), - SandboxPolicy::ReadOnly => false, + SandboxPolicy::ReadOnly { .. } => false, SandboxPolicy::WorkspaceWrite { network_access, .. } => *network_access, } } + /// Returns the list of readable roots (tailored to the current working + /// directory) when read access is restricted. + /// + /// For policies with full read access, this returns an empty list because + /// callers should grant blanket reads. + pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec { + let mut roots = match self { + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => Vec::new(), + SandboxPolicy::ReadOnly { access } => access.get_readable_roots_with_cwd(cwd), + SandboxPolicy::WorkspaceWrite { + writable_roots, + read_only_access, + .. + } => { + let mut roots = read_only_access.get_readable_roots_with_cwd(cwd); + roots.extend(writable_roots.clone()); + roots + } + }; + let mut seen = HashSet::new(); + roots.retain(|root| seen.insert(root.to_path_buf())); + roots + } + /// Returns the list of writable roots (tailored to the current working /// directory) together with subpaths that should remain read‑only under /// each writable root. @@ -518,9 +667,10 @@ impl SandboxPolicy { match self { SandboxPolicy::DangerFullAccess => Vec::new(), SandboxPolicy::ExternalSandbox { .. } => Vec::new(), - SandboxPolicy::ReadOnly => Vec::new(), + SandboxPolicy::ReadOnly { .. } => Vec::new(), SandboxPolicy::WorkspaceWrite { writable_roots, + read_only_access: _, exclude_tmpdir_env_var, exclude_slash_tmp, network_access: _, @@ -2705,7 +2855,7 @@ mod tests { model: "codex-mini-latest".to_string(), model_provider_id: "openai".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, diff --git a/codex-rs/state/src/model/thread_metadata.rs b/codex-rs/state/src/model/thread_metadata.rs index 2577ead502..d29bf07606 100644 --- a/codex-rs/state/src/model/thread_metadata.rs +++ b/codex-rs/state/src/model/thread_metadata.rs @@ -138,7 +138,7 @@ impl ThreadMetadataBuilder { model_provider: None, cwd: PathBuf::new(), cli_version: None, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), approval_mode: AskForApproval::OnRequest, archived_at: None, git_sha: None, diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index 541e02ad38..7110f29282 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -1997,7 +1997,7 @@ WHERE kind = 'memory_stage1' cwd, cli_version: "0.0.0".to_string(), title: String::new(), - sandbox_policy: crate::extract::enum_to_string(&SandboxPolicy::ReadOnly), + sandbox_policy: crate::extract::enum_to_string(&SandboxPolicy::new_read_only_policy()), approval_mode: crate::extract::enum_to_string(&AskForApproval::OnRequest), tokens_used: 0, first_user_message: Some("hello".to_string()), diff --git a/codex-rs/tui/src/additional_dirs.rs b/codex-rs/tui/src/additional_dirs.rs index 54746c1705..98724aa161 100644 --- a/codex-rs/tui/src/additional_dirs.rs +++ b/codex-rs/tui/src/additional_dirs.rs @@ -16,7 +16,7 @@ pub fn add_dir_warning_message( SandboxPolicy::WorkspaceWrite { .. } | SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => None, - SandboxPolicy::ReadOnly => Some(format_warning(additional_dirs)), + SandboxPolicy::ReadOnly { .. } => Some(format_warning(additional_dirs)), } } @@ -64,7 +64,7 @@ mod tests { #[test] fn warns_for_read_only() { - let sandbox = SandboxPolicy::ReadOnly; + let sandbox = SandboxPolicy::new_read_only_policy(); let dirs = vec![PathBuf::from("relative"), PathBuf::from("/abs")]; let message = add_dir_warning_message(&dirs, &sandbox) .expect("expected warning for read-only sandbox"); @@ -76,7 +76,7 @@ mod tests { #[test] fn returns_none_when_no_additional_dirs() { - let sandbox = SandboxPolicy::ReadOnly; + let sandbox = SandboxPolicy::new_read_only_policy(); let dirs: Vec = Vec::new(); assert_eq!(add_dir_warning_message(&dirs, &sandbox), None); } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 1ef3475e44..f2d0a49d28 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1156,7 +1156,7 @@ impl App { && matches!( app.config.sandbox_policy.get(), codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } - | codex_core::protocol::SandboxPolicy::ReadOnly + | codex_core::protocol::SandboxPolicy::ReadOnly { .. } ) && !app .config @@ -1982,7 +1982,7 @@ impl App { let policy_is_workspace_write_or_ro = matches!( &policy, codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. } - | codex_core::protocol::SandboxPolicy::ReadOnly + | codex_core::protocol::SandboxPolicy::ReadOnly { .. } ); if let Err(err) = self.config.sandbox_policy.set(policy.clone()) { @@ -1992,9 +1992,11 @@ impl App { return Ok(AppRunControl::Continue); } #[cfg(target_os = "windows")] - if !matches!(&policy, codex_core::protocol::SandboxPolicy::ReadOnly) - || WindowsSandboxLevel::from_config(&self.config) - != WindowsSandboxLevel::Disabled + if !matches!( + &policy, + codex_core::protocol::SandboxPolicy::ReadOnly { .. } + ) || WindowsSandboxLevel::from_config(&self.config) + != WindowsSandboxLevel::Disabled { self.config.forced_auto_mode_downgraded_on_windows = false; } @@ -3047,7 +3049,7 @@ mod tests { model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, @@ -3102,7 +3104,7 @@ mod tests { model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, @@ -3149,7 +3151,7 @@ mod tests { model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, history_log_id: 0, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e507a39a4d..4d5e05b5ac 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -5408,15 +5408,16 @@ impl ChatWidget { } matches!( (&preset.sandbox, current_sandbox), - (SandboxPolicy::ReadOnly, SandboxPolicy::ReadOnly) - | ( - SandboxPolicy::DangerFullAccess, - SandboxPolicy::DangerFullAccess - ) - | ( - SandboxPolicy::WorkspaceWrite { .. }, - SandboxPolicy::WorkspaceWrite { .. } - ) + ( + SandboxPolicy::ReadOnly { .. }, + SandboxPolicy::ReadOnly { .. } + ) | ( + SandboxPolicy::DangerFullAccess, + SandboxPolicy::DangerFullAccess + ) | ( + SandboxPolicy::WorkspaceWrite { .. }, + SandboxPolicy::WorkspaceWrite { .. } + ) ) } @@ -5537,7 +5538,7 @@ impl ChatWidget { let mut header_children: Vec> = Vec::new(); let describe_policy = |policy: &SandboxPolicy| match policy { SandboxPolicy::WorkspaceWrite { .. } => "Agent mode", - SandboxPolicy::ReadOnly => "Read-Only mode", + SandboxPolicy::ReadOnly { .. } => "Read-Only mode", _ => "Agent mode", }; let mode_label = preset @@ -5949,7 +5950,7 @@ impl ChatWidget { /// Set the sandbox policy in the widget's config copy. pub(crate) fn set_sandbox_policy(&mut self, policy: SandboxPolicy) -> ConstraintResult<()> { #[cfg(target_os = "windows")] - let should_clear_downgrade = !matches!(&policy, SandboxPolicy::ReadOnly) + let should_clear_downgrade = !matches!(&policy, SandboxPolicy::ReadOnly { .. }) || WindowsSandboxLevel::from_config(&self.config) != WindowsSandboxLevel::Disabled; self.config.sandbox_policy.set(policy)?; diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 1f1af74a11..fb25306b1c 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -148,7 +148,7 @@ async fn resumed_initial_messages_render_history() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -216,7 +216,7 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -334,7 +334,7 @@ async fn submission_preserves_text_elements_and_local_images() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -414,7 +414,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -2972,7 +2972,7 @@ async fn plan_slash_command_with_args_submits_prompt_in_plan_mode() { model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, - sandbox_policy: SandboxPolicy::ReadOnly, + sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: Some(ReasoningEffortConfig::default()), history_log_id: 0, @@ -4032,6 +4032,7 @@ async fn preset_matching_ignores_extra_writable_roots() { .expect("auto preset exists"); let current_sandbox = SandboxPolicy::WorkspaceWrite { writable_roots: vec![AbsolutePathBuf::try_from("C:\\extra").unwrap()], + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index 681d9c3a9a..80e97d5dd8 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -403,7 +403,7 @@ mod tests { Some(RequirementSource::CloudRequirements), ); requirements.sandbox_policy = ConstrainedWithSource::new( - Constrained::allow_any(SandboxPolicy::ReadOnly), + Constrained::allow_any(SandboxPolicy::new_read_only_policy()), Some(RequirementSource::SystemRequirementsToml { file: requirements_file.clone(), }), diff --git a/codex-rs/tui/src/status/card.rs b/codex-rs/tui/src/status/card.rs index 23005274db..61421976f5 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -157,7 +157,7 @@ impl StatusHistoryCell { .unwrap_or_else(|| "".to_string()); let sandbox = match config.sandbox_policy.get() { SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), - SandboxPolicy::ReadOnly => "read-only".to_string(), + SandboxPolicy::ReadOnly { .. } => "read-only".to_string(), SandboxPolicy::WorkspaceWrite { .. } => "workspace-write".to_string(), SandboxPolicy::ExternalSandbox { network_access } => { if matches!(network_access, NetworkAccess::Enabled) { diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index c8844904ab..c9bd2c6e67 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -100,6 +100,7 @@ async fn status_snapshot_includes_reasoning_details() { .sandbox_policy .set(SandboxPolicy::WorkspaceWrite { writable_roots: Vec::new(), + read_only_access: Default::default(), network_access: false, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index 1af9f1b919..2aefb7a3fd 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -262,7 +262,7 @@ pub fn apply_capability_denies_for_world_writable( } (sid, roots) } - SandboxPolicy::ReadOnly => ( + SandboxPolicy::ReadOnly { .. } => ( unsafe { convert_string_sid_to_sid(&caps.readonly) }.ok_or_else(|| { anyhow!("ConvertStringSidToSidW failed for readonly capability") })?, diff --git a/codex-rs/windows-sandbox-rs/src/command_runner_win.rs b/codex-rs/windows-sandbox-rs/src/command_runner_win.rs index 9ff61de932..da949f8778 100644 --- a/codex-rs/windows-sandbox-rs/src/command_runner_win.rs +++ b/codex-rs/windows-sandbox-rs/src/command_runner_win.rs @@ -114,6 +114,11 @@ pub fn main() -> Result<()> { ); let policy = parse_policy(&req.policy_json_or_preset).context("parse policy_json_or_preset")?; + if !policy.has_full_disk_read_access() { + anyhow::bail!( + "Restricted read-only access is not yet supported by the Windows sandbox backend" + ); + } let mut cap_psids: Vec<*mut c_void> = Vec::new(); for sid in &req.cap_sids { let Some(psid) = (unsafe { convert_string_sid_to_sid(sid) }) else { @@ -129,7 +134,9 @@ pub fn main() -> Result<()> { let base = unsafe { get_current_token_for_restriction()? }; let token_res: Result = unsafe { match &policy { - SandboxPolicy::ReadOnly => create_readonly_token_with_caps_from(base, &cap_psids), + SandboxPolicy::ReadOnly { .. } => { + create_readonly_token_with_caps_from(base, &cap_psids) + } SandboxPolicy::WorkspaceWrite { .. } => { create_workspace_write_token_with_caps_from(base, &cap_psids) } diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index ecd5732c09..bc96599ed0 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -238,9 +238,14 @@ mod windows_impl { ) { anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing") } + if !policy.has_full_disk_read_access() { + anyhow::bail!( + "Restricted read-only access is not yet supported by the Windows sandbox backend" + ); + } let caps = load_or_create_cap_sids(codex_home)?; let (psid_to_use, cap_sids) = match &policy { - SandboxPolicy::ReadOnly => ( + SandboxPolicy::ReadOnly { .. } => ( unsafe { convert_string_sid_to_sid(&caps.readonly).unwrap() }, vec![caps.readonly.clone()], ), @@ -487,7 +492,7 @@ mod windows_impl { #[test] fn applies_network_block_for_read_only() { - assert!(!SandboxPolicy::ReadOnly.has_full_network_access()); + assert!(!SandboxPolicy::new_read_only_policy().has_full_network_access()); } } } diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 4a97efaaf8..5d71b4b206 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -258,10 +258,15 @@ mod windows_impl { ) { anyhow::bail!("DangerFullAccess and ExternalSandbox are not supported for sandboxing") } + if !policy.has_full_disk_read_access() { + anyhow::bail!( + "Restricted read-only access is not yet supported by the Windows sandbox backend" + ); + } let caps = load_or_create_cap_sids(codex_home)?; let (h_token, psid_generic, psid_workspace): (HANDLE, *mut c_void, Option<*mut c_void>) = unsafe { match &policy { - SandboxPolicy::ReadOnly => { + SandboxPolicy::ReadOnly { .. } => { let psid = convert_string_sid_to_sid(&caps.readonly).unwrap(); let (h, _) = super::token::create_readonly_token_with_cap(psid)?; (h, psid, None) @@ -528,7 +533,9 @@ mod windows_impl { #[test] fn applies_network_block_for_read_only() { - assert!(should_apply_network_block(&SandboxPolicy::ReadOnly)); + assert!(should_apply_network_block( + &SandboxPolicy::new_read_only_policy() + )); } } } diff --git a/codex-rs/windows-sandbox-rs/src/policy.rs b/codex-rs/windows-sandbox-rs/src/policy.rs index 64fc56052f..3cee37cdf7 100644 --- a/codex-rs/windows-sandbox-rs/src/policy.rs +++ b/codex-rs/windows-sandbox-rs/src/policy.rs @@ -3,7 +3,7 @@ pub use codex_protocol::protocol::SandboxPolicy; pub fn parse_policy(value: &str) -> Result { match value { - "read-only" => Ok(SandboxPolicy::ReadOnly), + "read-only" => Ok(SandboxPolicy::new_read_only_policy()), "workspace-write" => Ok(SandboxPolicy::new_workspace_write_policy()), "danger-full-access" | "external-sandbox" => anyhow::bail!( "DangerFullAccess and ExternalSandbox are not supported for sandboxing" @@ -52,6 +52,9 @@ mod tests { #[test] fn parses_read_only_policy() { - assert_eq!(parse_policy("read-only").unwrap(), SandboxPolicy::ReadOnly); + assert_eq!( + parse_policy("read-only").unwrap(), + SandboxPolicy::new_read_only_policy() + ); } }