From 71f1c6ed79edffe900ef216e216e8595753d2de5 Mon Sep 17 00:00:00 2001 From: Roy Han Date: Tue, 17 Mar 2026 17:22:53 -0700 Subject: [PATCH] persist untruncated approve rule list --- codex-rs/app-server/tests/common/rollout.rs | 2 + .../tests/suite/v2/thread_resume.rs | 1 + codex-rs/core/src/auth_env_telemetry.rs | 1 + codex-rs/core/src/codex.rs | 11 +++ codex-rs/core/src/codex_tests.rs | 3 + .../core/src/personality_migration_tests.rs | 1 + codex-rs/core/src/rollout/metadata_tests.rs | 4 + codex-rs/core/src/rollout/recorder.rs | 6 ++ codex-rs/core/src/rollout/recorder_tests.rs | 16 ++++ codex-rs/core/src/rollout/tests.rs | 1 + .../core/tests/suite/personality_migration.rs | 2 + .../core/tests/suite/rollout_list_find.rs | 1 + codex-rs/core/tests/suite/sqlite_state.rs | 1 + codex-rs/protocol/src/models.rs | 76 ++++++++++++++----- codex-rs/protocol/src/protocol.rs | 11 +++ codex-rs/state/src/extract.rs | 2 + codex-rs/state/src/runtime/threads.rs | 2 + 17 files changed, 120 insertions(+), 21 deletions(-) diff --git a/codex-rs/app-server/tests/common/rollout.rs b/codex-rs/app-server/tests/common/rollout.rs index 8146f7ae93..34baed8ab6 100644 --- a/codex-rs/app-server/tests/common/rollout.rs +++ b/codex-rs/app-server/tests/common/rollout.rs @@ -85,6 +85,7 @@ pub fn create_fake_rollout_with_source( base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }; let payload = serde_json::to_value(SessionMetaLine { meta, @@ -167,6 +168,7 @@ pub fn create_fake_rollout_with_text_elements( base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }; let payload = serde_json::to_value(SessionMetaLine { meta, diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 27803db42c..718eef5fb5 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -328,6 +328,7 @@ stream_max_retries = 0 base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }; std::fs::write( &rollout_path, diff --git a/codex-rs/core/src/auth_env_telemetry.rs b/codex-rs/core/src/auth_env_telemetry.rs index be281e05a1..85cd23fe06 100644 --- a/codex-rs/core/src/auth_env_telemetry.rs +++ b/codex-rs/core/src/auth_env_telemetry.rs @@ -71,6 +71,7 @@ mod tests { request_max_retries: None, stream_max_retries: None, stream_idle_timeout_ms: None, + websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, }; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index d367479187..8c46482763 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -91,9 +91,11 @@ use codex_protocol::mcp::CallToolResult; use codex_protocol::models::BaseInstructions; use codex_protocol::models::PermissionProfile; use codex_protocol::models::format_allow_prefixes; +use codex_protocol::models::summarize_allow_prefixes; use codex_protocol::openai_models::ModelInfo; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::ApprovedCommandPrefixesSnapshot; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::HasLegacyEvent; use codex_protocol::protocol::ItemCompletedEvent; @@ -1404,6 +1406,9 @@ impl Session { let (conversation_id, rollout_params) = match &initial_history { InitialHistory::New | InitialHistory::Forked(_) => { let conversation_id = ThreadId::default(); + let approved_command_prefixes = exec_policy.current().get_allowed_prefixes(); + let allow_prefix_summary = + summarize_allow_prefixes(approved_command_prefixes.clone()); ( conversation_id, RolloutRecorderParams::new( @@ -1414,6 +1419,12 @@ impl Session { text: session_configuration.base_instructions.clone(), }, session_configuration.dynamic_tools.clone(), + ApprovedCommandPrefixesSnapshot { + prefixes: approved_command_prefixes, + prompt_truncated: allow_prefix_summary.prompt_truncated, + total_count: allow_prefix_summary.total_count, + prompt_visible_count: allow_prefix_summary.prompt_visible_count, + }, if session_configuration.persist_extended_history { EventPersistenceMode::Extended } else { diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 0f45e3de66..f4aad70ef9 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2008,6 +2008,7 @@ async fn attach_rollout_recorder(session: &Arc) -> PathBuf { SessionSource::Exec, BaseInstructions::default(), Vec::new(), + Default::default(), EventPersistenceMode::Limited, ), None, @@ -3902,6 +3903,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_baseline SessionSource::Exec, BaseInstructions::default(), Vec::new(), + Default::default(), EventPersistenceMode::Limited, ), None, @@ -3999,6 +4001,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_full_rei SessionSource::Exec, BaseInstructions::default(), Vec::new(), + Default::default(), EventPersistenceMode::Limited, ), None, diff --git a/codex-rs/core/src/personality_migration_tests.rs b/codex-rs/core/src/personality_migration_tests.rs index fef1297a97..ea438ca6e2 100644 --- a/codex-rs/core/src/personality_migration_tests.rs +++ b/codex-rs/core/src/personality_migration_tests.rs @@ -44,6 +44,7 @@ async fn write_session_with_user_event(codex_home: &Path) -> io::Result<()> { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }, git: None, }; diff --git a/codex-rs/core/src/rollout/metadata_tests.rs b/codex-rs/core/src/rollout/metadata_tests.rs index 5556d7002d..6d34dc26b7 100644 --- a/codex-rs/core/src/rollout/metadata_tests.rs +++ b/codex-rs/core/src/rollout/metadata_tests.rs @@ -44,6 +44,7 @@ async fn extract_metadata_from_rollout_uses_session_meta() { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }; let session_meta_line = SessionMetaLine { meta: session_meta, @@ -94,9 +95,11 @@ async fn extract_metadata_from_rollout_returns_latest_memory_mode() { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }; let polluted_meta = SessionMeta { memory_mode: Some("polluted".to_string()), + approved_command_prefixes: None, ..session_meta.clone() }; let lines = vec![ @@ -361,6 +364,7 @@ fn write_rollout_in_sessions_with_cwd( base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }; let session_meta_line = SessionMetaLine { meta: session_meta, diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 002269d59e..25c270de45 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -47,6 +47,7 @@ use crate::state_db; use crate::state_db::StateDbHandle; use crate::truncate::TruncationPolicy; use crate::truncate::truncate_text; +use codex_protocol::protocol::ApprovedCommandPrefixesSnapshot; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::ResumedHistory; @@ -83,6 +84,7 @@ pub enum RolloutRecorderParams { source: SessionSource, base_instructions: BaseInstructions, dynamic_tools: Vec, + approved_command_prefixes: ApprovedCommandPrefixesSnapshot, event_persistence_mode: EventPersistenceMode, }, Resume { @@ -112,6 +114,7 @@ impl RolloutRecorderParams { source: SessionSource, base_instructions: BaseInstructions, dynamic_tools: Vec, + approved_command_prefixes: ApprovedCommandPrefixesSnapshot, event_persistence_mode: EventPersistenceMode, ) -> Self { Self::Create { @@ -120,6 +123,7 @@ impl RolloutRecorderParams { source, base_instructions, dynamic_tools, + approved_command_prefixes, event_persistence_mode, } } @@ -381,6 +385,7 @@ impl RolloutRecorder { source, base_instructions, dynamic_tools, + approved_command_prefixes, event_persistence_mode, } => { let log_file_info = precompute_log_file_info(config, conversation_id)?; @@ -415,6 +420,7 @@ impl RolloutRecorder { }, memory_mode: (!config.memories.generate_memories) .then_some("disabled".to_string()), + approved_command_prefixes: Some(approved_command_prefixes), }; ( diff --git a/codex-rs/core/src/rollout/recorder_tests.rs b/codex-rs/core/src/rollout/recorder_tests.rs index f6f588574a..52cead980b 100644 --- a/codex-rs/core/src/rollout/recorder_tests.rs +++ b/codex-rs/core/src/rollout/recorder_tests.rs @@ -4,6 +4,7 @@ use crate::features::Feature; use chrono::TimeZone; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::protocol::AgentMessageEvent; +use codex_protocol::protocol::ApprovedCommandPrefixesSnapshot; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::SandboxPolicy; @@ -59,6 +60,15 @@ async fn recorder_materializes_only_after_explicit_persist() -> std::io::Result< .build() .await?; let thread_id = ThreadId::new(); + let approved_command_prefixes = ApprovedCommandPrefixesSnapshot { + prefixes: vec![ + vec!["cargo".to_string(), "test".to_string()], + vec!["git".to_string(), "fetch".to_string()], + ], + prompt_truncated: true, + total_count: 101, + prompt_visible_count: 100, + }; let recorder = RolloutRecorder::new( &config, RolloutRecorderParams::new( @@ -67,6 +77,7 @@ async fn recorder_materializes_only_after_explicit_persist() -> std::io::Result< SessionSource::Exec, BaseInstructions::default(), Vec::new(), + approved_command_prefixes.clone(), EventPersistenceMode::Limited, ), None, @@ -120,6 +131,10 @@ async fn recorder_materializes_only_after_explicit_persist() -> std::io::Result< text.contains("\"type\":\"session_meta\""), "expected session metadata in rollout" ); + assert!( + text.contains(&serde_json::to_string(&approved_command_prefixes)?), + "expected approved command prefixes snapshot in rollout" + ); let buffered_idx = text .find("buffered-event") .expect("buffered event in rollout"); @@ -166,6 +181,7 @@ async fn metadata_irrelevant_events_touch_state_db_updated_at() -> std::io::Resu SessionSource::Cli, BaseInstructions::default(), Vec::new(), + Default::default(), EventPersistenceMode::Limited, ), Some(state_db.clone()), diff --git a/codex-rs/core/src/rollout/tests.rs b/codex-rs/core/src/rollout/tests.rs index 12af36b783..6039c91ca0 100644 --- a/codex-rs/core/src/rollout/tests.rs +++ b/codex-rs/core/src/rollout/tests.rs @@ -1107,6 +1107,7 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }, git: None, }), diff --git a/codex-rs/core/tests/suite/personality_migration.rs b/codex-rs/core/tests/suite/personality_migration.rs index adbd86cb23..778b309d4a 100644 --- a/codex-rs/core/tests/suite/personality_migration.rs +++ b/codex-rs/core/tests/suite/personality_migration.rs @@ -72,6 +72,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }, git: None, }; @@ -116,6 +117,7 @@ async fn write_rollout_with_meta_only(dir: &Path, thread_id: ThreadId) -> io::Re base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }, git: None, }; diff --git a/codex-rs/core/tests/suite/rollout_list_find.rs b/codex-rs/core/tests/suite/rollout_list_find.rs index a9ea2b9c8a..24361bfd60 100644 --- a/codex-rs/core/tests/suite/rollout_list_find.rs +++ b/codex-rs/core/tests/suite/rollout_list_find.rs @@ -172,6 +172,7 @@ async fn find_locates_rollout_file_written_by_recorder() -> std::io::Result<()> SessionSource::Exec, BaseInstructions::default(), Vec::new(), + Default::default(), EventPersistenceMode::Limited, ), None, diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 0252f3e086..21423006d3 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -147,6 +147,7 @@ async fn backfill_scans_existing_rollouts() -> Result<()> { base_instructions: None, dynamic_tools: Some(dynamic_tools_for_hook), memory_mode: None, + approved_command_prefixes: None, }, git: None, }; diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 1e5a7445e3..9be144ea6a 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -778,13 +778,18 @@ const MAX_RENDERED_PREFIXES: usize = 100; const MAX_ALLOW_PREFIX_TEXT_BYTES: usize = 5000; const TRUNCATED_MARKER: &str = "...\n[Some commands were truncated]"; -pub fn format_allow_prefixes(prefixes: Vec>) -> Option { - let mut truncated = false; - if prefixes.len() > MAX_RENDERED_PREFIXES { - truncated = true; - } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AllowPrefixRenderSummary { + pub rendered: String, + pub prompt_truncated: bool, + pub total_count: usize, + pub prompt_visible_count: usize, +} + +pub fn summarize_allow_prefixes(mut prefixes: Vec>) -> AllowPrefixRenderSummary { + let total_count = prefixes.len(); + let mut truncated = total_count > MAX_RENDERED_PREFIXES; - let mut prefixes = prefixes; prefixes.sort_by(|a, b| { a.len() .cmp(&b.len()) @@ -792,15 +797,15 @@ pub fn format_allow_prefixes(prefixes: Vec>) -> Option { .then_with(|| a.cmp(b)) }); - let full_text = prefixes + let rendered_prefixes = prefixes .into_iter() .take(MAX_RENDERED_PREFIXES) .map(|prefix| format!("- {}", render_command_prefix(&prefix))) - .collect::>() - .join("\n"); + .collect::>(); + let mut output = rendered_prefixes.join("\n"); + let mut prompt_visible_count = rendered_prefixes.len(); // truncate to last UTF8 char - let mut output = full_text; let byte_idx = output .char_indices() .nth(MAX_ALLOW_PREFIX_TEXT_BYTES) @@ -808,13 +813,27 @@ pub fn format_allow_prefixes(prefixes: Vec>) -> Option { if let Some(byte_idx) = byte_idx { truncated = true; output = output[..byte_idx].to_string(); + prompt_visible_count = if output.is_empty() { + 0 + } else { + output.lines().count() + }; } if truncated { - Some(format!("{output}{TRUNCATED_MARKER}")) - } else { - Some(output) + output = format!("{output}{TRUNCATED_MARKER}"); } + + AllowPrefixRenderSummary { + rendered: output, + prompt_truncated: truncated, + total_count, + prompt_visible_count, + } +} + +pub fn format_allow_prefixes(prefixes: Vec>) -> Option { + Some(summarize_allow_prefixes(prefixes).rendered) } fn prefix_combined_str_len(prefix: &[String]) -> usize { @@ -2280,10 +2299,11 @@ mod tests { .map(|i| vec![format!("{i:03}")]) .collect::>(); - let output = format_allow_prefixes(prefixes).expect("rendered list"); - assert_eq!(output.ends_with(TRUNCATED_MARKER), true); - eprintln!("output: {output}"); - assert_eq!(output.lines().count(), MAX_RENDERED_PREFIXES + 1); + let summary = summarize_allow_prefixes(prefixes); + assert!(summary.rendered.ends_with(TRUNCATED_MARKER)); + assert_eq!(summary.total_count, MAX_RENDERED_PREFIXES + 5); + assert_eq!(summary.prompt_visible_count, MAX_RENDERED_PREFIXES); + assert_eq!(summary.rendered.lines().count(), MAX_RENDERED_PREFIXES + 1); } #[test] @@ -2298,12 +2318,26 @@ mod tests { .expect("add rule"); } - let output = - format_allow_prefixes(exec_policy.get_allowed_prefixes()).expect("formatted prefixes"); + let summary = summarize_allow_prefixes(exec_policy.get_allowed_prefixes()); assert!( - output.len() <= MAX_ALLOW_PREFIX_TEXT_BYTES + TRUNCATED_MARKER.len(), - "output length exceeds expected limit: {output}", + summary.rendered.len() <= MAX_ALLOW_PREFIX_TEXT_BYTES + TRUNCATED_MARKER.len(), + "output length exceeds expected limit: {}", + summary.rendered, ); + assert!(summary.prompt_truncated); + assert!(summary.prompt_visible_count < summary.total_count); + } + + #[test] + fn summarize_allow_prefixes_reports_untruncated_counts() { + let summary = summarize_allow_prefixes(vec![ + vec!["git".to_string(), "fetch".to_string()], + vec!["cargo".to_string(), "test".to_string()], + ]); + + assert_eq!(summary.total_count, 2); + assert_eq!(summary.prompt_visible_count, 2); + assert!(!summary.prompt_truncated); } #[test] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 69f71d4b0e..8e0f4459d8 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -2344,6 +2344,14 @@ impl fmt::Display for SubAgentSource { /// NOTE: There used to be an `instructions` field here, which stored user_instructions, but we /// now save that on TurnContext. base_instructions stores the base instructions for the session, /// and should be used when there is no config override. +#[derive(Default, Serialize, Deserialize, Clone, Debug, PartialEq, Eq, JsonSchema, TS)] +pub struct ApprovedCommandPrefixesSnapshot { + pub prefixes: Vec>, + pub prompt_truncated: bool, + pub total_count: usize, + pub prompt_visible_count: usize, +} + #[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)] pub struct SessionMeta { pub id: ThreadId, @@ -2370,6 +2378,8 @@ pub struct SessionMeta { pub dynamic_tools: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub memory_mode: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub approved_command_prefixes: Option, } impl Default for SessionMeta { @@ -2388,6 +2398,7 @@ impl Default for SessionMeta { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, } } } diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 037b1f5d22..a8a416ec1a 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -257,6 +257,7 @@ mod tests { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }, git: None, }), @@ -383,6 +384,7 @@ mod tests { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }, git: None, }), diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index 6373568e26..4a19ce92fe 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -764,6 +764,7 @@ mod tests { base_instructions: None, dynamic_tools: None, memory_mode: Some("polluted".to_string()), + approved_command_prefixes: None, }, git: None, })]; @@ -818,6 +819,7 @@ mod tests { base_instructions: None, dynamic_tools: None, memory_mode: None, + approved_command_prefixes: None, }, git: Some(GitInfo { commit_hash: Some("rollout-sha".to_string()),