diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 0b52d2ce6..56daf910e 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -115,10 +115,7 @@ Example with notification opt-out: }, "capabilities": { "experimentalApi": true, - "optOutNotificationMethods": [ - "thread/started", - "item/agentMessage/delta" - ] + "optOutNotificationMethods": ["thread/started", "item/agentMessage/delta"] } } } @@ -228,7 +225,11 @@ Start a fresh thread when you need a new Codex conversation. Valid `personality` values are `"friendly"`, `"pragmatic"`, and `"none"`. When `"none"` is selected, the personality placeholder is replaced with an empty string. -To continue a stored session, call `thread/resume` with the `thread.id` you previously recorded. The response shape matches `thread/start`, and no additional notifications are emitted. You can also pass the same configuration overrides supported by `thread/start`, including `approvalsReviewer`: +To continue a stored session, call `thread/resume` with the `thread.id` you previously recorded. The response shape matches `thread/start`, and no additional notifications are emitted. You can also pass the same configuration overrides supported by `thread/start`, including `approvalsReviewer`. + +By default, resume uses the latest persisted `model` and `reasoningEffort` values associated with the thread. Supplying any of `model`, `modelProvider`, `config.model`, or `config.model_reasoning_effort` disables that persisted fallback and uses the explicit overrides plus normal config resolution instead. + +Example: ```json { "method": "thread/resume", "id": 11, "params": { @@ -301,10 +302,13 @@ When `nextCursor` is `null`, you’ve reached the final page. - `thread/start`, `thread/fork`, and detached review threads do not emit a separate initial `thread/status/changed`; their `thread/started` notification already carries the current `thread.status`. ```json -{ "method": "thread/status/changed", "params": { +{ + "method": "thread/status/changed", + "params": { "threadId": "thr_123", "status": { "type": "active", "activeFlags": [] } -} } + } +} ``` ### Example: Unsubscribe from a loaded thread @@ -953,10 +957,7 @@ The built-in `request_permissions` tool sends an `item/permissions/requestApprov "reason": "Select a workspace root", "permissions": { "fileSystem": { - "write": [ - "/Users/me/project", - "/Users/me/shared" - ] + "write": ["/Users/me/project", "/Users/me/shared"] } } } @@ -972,9 +973,7 @@ The client responds with `result.permissions`, which should be the granted subse "scope": "session", "permissions": { "fileSystem": { - "write": [ - "/Users/me/project" - ] + "write": ["/Users/me/project"] } } } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c70be7e8e..d3eb1df79 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -271,6 +271,7 @@ use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS; use codex_protocol::user_input::UserInput as CoreInputItem; use codex_rmcp_client::perform_oauth_login_return_url; use codex_state::StateRuntime; +use codex_state::ThreadMetadata; use codex_state::ThreadMetadataBuilder; use codex_state::log_db::LogDbLayer; use codex_utils_json_to_toml::json_to_toml; @@ -3346,7 +3347,7 @@ impl CodexMessageProcessor { approval_policy, approvals_reviewer, sandbox, - config: request_overrides, + config: mut request_overrides, base_instructions, developer_instructions, personality, @@ -3372,7 +3373,7 @@ impl CodexMessageProcessor { }; let history_cwd = thread_history.session_cwd(); - let typesafe_overrides = self.build_thread_config_overrides( + let mut typesafe_overrides = self.build_thread_config_overrides( model, model_provider, service_tier, @@ -3384,6 +3385,13 @@ impl CodexMessageProcessor { developer_instructions, personality, ); + let persisted_resume_metadata = self + .load_and_apply_persisted_resume_metadata( + &thread_history, + &mut request_overrides, + &mut typesafe_overrides, + ) + .await; // Derive a Config using the same logic as new conversation, honoring overrides if provided. let cloud_requirements = self.current_cloud_requirements(); @@ -3447,18 +3455,22 @@ impl CodexMessageProcessor { "thread", ); - let Some(mut thread) = self + let mut thread = match self .load_thread_from_resume_source_or_send_internal( - request_id.clone(), thread_id, thread.as_ref(), &response_history, rollout_path.as_path(), fallback_model_provider.as_str(), + persisted_resume_metadata.as_ref(), ) .await - else { - return; + { + Ok(thread) => thread, + Err(message) => { + self.send_internal_error(request_id, message).await; + return; + } }; self.thread_watch_manager @@ -3501,6 +3513,25 @@ impl CodexMessageProcessor { } } + async fn load_and_apply_persisted_resume_metadata( + &self, + thread_history: &InitialHistory, + request_overrides: &mut Option>, + typesafe_overrides: &mut ConfigOverrides, + ) -> Option { + let InitialHistory::Resumed(resumed_history) = thread_history else { + return None; + }; + let state_db_ctx = get_state_db(&self.config).await?; + let persisted_metadata = state_db_ctx + .get_thread(resumed_history.conversation_id) + .await + .ok() + .flatten()?; + merge_persisted_resume_metadata(request_overrides, typesafe_overrides, &persisted_metadata); + Some(persisted_metadata) + } + async fn resume_running_thread( &mut self, request_id: ConnectionRequestId, @@ -3617,6 +3648,7 @@ impl CodexMessageProcessor { existing_thread_id, rollout_path.as_path(), config_snapshot.model_provider_id.as_str(), + /*persisted_metadata*/ None, ) .await { @@ -3748,13 +3780,13 @@ impl CodexMessageProcessor { async fn load_thread_from_resume_source_or_send_internal( &self, - request_id: ConnectionRequestId, thread_id: ThreadId, thread: &CodexThread, thread_history: &InitialHistory, rollout_path: &Path, fallback_provider: &str, - ) -> Option { + persisted_resume_metadata: Option<&ThreadMetadata>, + ) -> std::result::Result { let thread = match thread_history { InitialHistory::Resumed(resumed) => { load_thread_summary_for_rollout( @@ -3762,6 +3794,7 @@ impl CodexMessageProcessor { resumed.conversation_id, resumed.rollout_path.as_path(), fallback_provider, + persisted_resume_metadata, ) .await } @@ -3779,28 +3812,18 @@ impl CodexMessageProcessor { "failed to build resume response for thread {thread_id}: initial history missing" )), }; - let mut thread = match thread { - Ok(thread) => thread, - Err(message) => { - self.send_internal_error(request_id, message).await; - return None; - } - }; + let mut thread = thread?; thread.id = thread_id.to_string(); thread.path = Some(rollout_path.to_path_buf()); let history_items = thread_history.get_rollout_items(); - if let Err(message) = populate_thread_turns( + populate_thread_turns( &mut thread, ThreadTurnSource::HistoryItems(&history_items), /*active_turn*/ None, ) - .await - { - self.send_internal_error(request_id, message).await; - return None; - } + .await?; self.attach_thread_name(thread_id, &mut thread).await; - Some(thread) + Ok(thread) } async fn attach_thread_name(&self, thread_id: ThreadId, thread: &mut Thread) { @@ -7391,6 +7414,36 @@ fn collect_resume_override_mismatches( mismatch_details } +fn merge_persisted_resume_metadata( + request_overrides: &mut Option>, + typesafe_overrides: &mut ConfigOverrides, + persisted_metadata: &ThreadMetadata, +) { + if has_model_resume_override(request_overrides.as_ref(), typesafe_overrides) { + return; + } + + typesafe_overrides.model = persisted_metadata.model.clone(); + + if let Some(reasoning_effort) = persisted_metadata.reasoning_effort { + request_overrides.get_or_insert_with(HashMap::new).insert( + "model_reasoning_effort".to_string(), + serde_json::Value::String(reasoning_effort.to_string()), + ); + } +} + +fn has_model_resume_override( + request_overrides: Option<&HashMap>, + typesafe_overrides: &ConfigOverrides, +) -> bool { + typesafe_overrides.model.is_some() + || typesafe_overrides.model_provider.is_some() + || request_overrides.is_some_and(|overrides| overrides.contains_key("model")) + || request_overrides + .is_some_and(|overrides| overrides.contains_key("model_reasoning_effort")) +} + fn skills_to_info( skills: &[codex_core::skills::SkillMetadata], disabled_paths: &std::collections::HashSet, @@ -7705,26 +7758,7 @@ async fn read_summary_from_state_db_context_by_thread_id( Ok(Some(metadata)) => metadata, Ok(None) | Err(_) => return None, }; - Some(summary_from_state_db_metadata( - metadata.id, - metadata.rollout_path, - metadata.first_user_message, - metadata - .created_at - .to_rfc3339_opts(SecondsFormat::Secs, true), - metadata - .updated_at - .to_rfc3339_opts(SecondsFormat::Secs, true), - metadata.model_provider, - metadata.cwd, - metadata.cli_version, - metadata.source, - metadata.agent_nickname, - metadata.agent_role, - metadata.git_sha, - metadata.git_branch, - metadata.git_origin_url, - )) + Some(summary_from_thread_metadata(&metadata)) } async fn summary_from_thread_list_item( @@ -7835,6 +7869,29 @@ fn summary_from_state_db_metadata( } } +fn summary_from_thread_metadata(metadata: &ThreadMetadata) -> ConversationSummary { + summary_from_state_db_metadata( + metadata.id, + metadata.rollout_path.clone(), + metadata.first_user_message.clone(), + metadata + .created_at + .to_rfc3339_opts(SecondsFormat::Secs, true), + metadata + .updated_at + .to_rfc3339_opts(SecondsFormat::Secs, true), + metadata.model_provider.clone(), + metadata.cwd.clone(), + metadata.cli_version.clone(), + metadata.source.clone(), + metadata.agent_nickname.clone(), + metadata.agent_role.clone(), + metadata.git_sha.clone(), + metadata.git_branch.clone(), + metadata.git_origin_url.clone(), + ) +} + pub(crate) async fn read_summary_from_rollout( path: &Path, fallback_provider: &str, @@ -7982,6 +8039,7 @@ async fn load_thread_summary_for_rollout( thread_id: ThreadId, rollout_path: &Path, fallback_provider: &str, + persisted_metadata: Option<&ThreadMetadata>, ) -> std::result::Result { let mut thread = read_summary_from_rollout(rollout_path, fallback_provider) .await @@ -7992,7 +8050,12 @@ async fn load_thread_summary_for_rollout( rollout_path.display() ) })?; - if let Some(summary) = read_summary_from_state_db_by_thread_id(config, thread_id).await { + if let Some(persisted_metadata) = persisted_metadata { + merge_mutable_thread_metadata( + &mut thread, + summary_to_thread(summary_from_thread_metadata(persisted_metadata)), + ); + } else if let Some(summary) = read_summary_from_state_db_by_thread_id(config, thread_id).await { merge_mutable_thread_metadata(&mut thread, summary_to_thread(summary)); } Ok(thread) @@ -8144,6 +8207,7 @@ mod tests { use anyhow::Result; use codex_app_server_protocol::ServerRequestPayload; use codex_app_server_protocol::ToolRequestUserInputParams; + use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use pretty_assertions::assert_eq; @@ -8275,6 +8339,178 @@ mod tests { ); } + fn test_thread_metadata( + model: Option<&str>, + reasoning_effort: Option, + ) -> Result { + let thread_id = ThreadId::from_string("3f941c35-29b3-493b-b0a4-e25800d9aeb0")?; + let mut builder = ThreadMetadataBuilder::new( + thread_id, + PathBuf::from("/tmp/rollout.jsonl"), + Utc::now(), + codex_protocol::protocol::SessionSource::default(), + ); + builder.model_provider = Some("mock_provider".to_string()); + let mut metadata = builder.build("mock_provider"); + metadata.model = model.map(ToString::to_string); + metadata.reasoning_effort = reasoning_effort; + Ok(metadata) + } + + #[test] + fn merge_persisted_resume_metadata_prefers_persisted_model_and_reasoning_effort() -> Result<()> + { + let mut request_overrides = None; + let mut typesafe_overrides = ConfigOverrides::default(); + let persisted_metadata = + test_thread_metadata(Some("gpt-5.1-codex-max"), Some(ReasoningEffort::High))?; + + merge_persisted_resume_metadata( + &mut request_overrides, + &mut typesafe_overrides, + &persisted_metadata, + ); + + assert_eq!( + typesafe_overrides.model, + Some("gpt-5.1-codex-max".to_string()) + ); + assert_eq!( + request_overrides, + Some(HashMap::from([( + "model_reasoning_effort".to_string(), + serde_json::Value::String("high".to_string()), + )])) + ); + Ok(()) + } + + #[test] + fn merge_persisted_resume_metadata_preserves_explicit_overrides() -> Result<()> { + let mut request_overrides = Some(HashMap::from([( + "model_reasoning_effort".to_string(), + serde_json::Value::String("low".to_string()), + )])); + let mut typesafe_overrides = ConfigOverrides { + model: Some("gpt-5.2-codex".to_string()), + ..Default::default() + }; + let persisted_metadata = + test_thread_metadata(Some("gpt-5.1-codex-max"), Some(ReasoningEffort::High))?; + + merge_persisted_resume_metadata( + &mut request_overrides, + &mut typesafe_overrides, + &persisted_metadata, + ); + + assert_eq!(typesafe_overrides.model, Some("gpt-5.2-codex".to_string())); + assert_eq!( + request_overrides, + Some(HashMap::from([( + "model_reasoning_effort".to_string(), + serde_json::Value::String("low".to_string()), + )])) + ); + Ok(()) + } + + #[test] + fn merge_persisted_resume_metadata_skips_persisted_values_when_model_overridden() -> Result<()> + { + let mut request_overrides = Some(HashMap::from([( + "model".to_string(), + serde_json::Value::String("gpt-5.2-codex".to_string()), + )])); + let mut typesafe_overrides = ConfigOverrides::default(); + let persisted_metadata = + test_thread_metadata(Some("gpt-5.1-codex-max"), Some(ReasoningEffort::High))?; + + merge_persisted_resume_metadata( + &mut request_overrides, + &mut typesafe_overrides, + &persisted_metadata, + ); + + assert_eq!(typesafe_overrides.model, None); + assert_eq!( + request_overrides, + Some(HashMap::from([( + "model".to_string(), + serde_json::Value::String("gpt-5.2-codex".to_string()), + )])) + ); + Ok(()) + } + + #[test] + fn merge_persisted_resume_metadata_skips_persisted_values_when_provider_overridden() + -> Result<()> { + let mut request_overrides = None; + let mut typesafe_overrides = ConfigOverrides { + model_provider: Some("oss".to_string()), + ..Default::default() + }; + let persisted_metadata = + test_thread_metadata(Some("gpt-5.1-codex-max"), Some(ReasoningEffort::High))?; + + merge_persisted_resume_metadata( + &mut request_overrides, + &mut typesafe_overrides, + &persisted_metadata, + ); + + assert_eq!(typesafe_overrides.model, None); + assert_eq!(typesafe_overrides.model_provider, Some("oss".to_string())); + assert_eq!(request_overrides, None); + Ok(()) + } + + #[test] + fn merge_persisted_resume_metadata_skips_persisted_values_when_reasoning_effort_overridden() + -> Result<()> { + let mut request_overrides = Some(HashMap::from([( + "model_reasoning_effort".to_string(), + serde_json::Value::String("low".to_string()), + )])); + let mut typesafe_overrides = ConfigOverrides::default(); + let persisted_metadata = + test_thread_metadata(Some("gpt-5.1-codex-max"), Some(ReasoningEffort::High))?; + + merge_persisted_resume_metadata( + &mut request_overrides, + &mut typesafe_overrides, + &persisted_metadata, + ); + + assert_eq!(typesafe_overrides.model, None); + assert_eq!( + request_overrides, + Some(HashMap::from([( + "model_reasoning_effort".to_string(), + serde_json::Value::String("low".to_string()), + )])) + ); + Ok(()) + } + + #[test] + fn merge_persisted_resume_metadata_skips_missing_values() -> Result<()> { + let mut request_overrides = None; + let mut typesafe_overrides = ConfigOverrides::default(); + let persisted_metadata = test_thread_metadata(None, None)?; + + merge_persisted_resume_metadata( + &mut request_overrides, + &mut typesafe_overrides, + &persisted_metadata, + ); + + assert_eq!(typesafe_overrides.model, None); + assert_eq!(request_overrides, None); + Ok(()) + } + #[test] fn extract_conversation_summary_prefers_plain_user_messages() -> Result<()> { let conversation_id = ThreadId::from_string("3f941c35-29b3-493b-b0a4-e25800d9aeb0")?;