diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2eeafe30b9..3d55019cde 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2985,6 +2985,7 @@ dependencies = [ "codex-protocol", "codex-state", "codex-tools", + "codex-utils-template", "pretty_assertions", "serde", "serde_json", @@ -3590,6 +3591,7 @@ dependencies = [ "tokio", "tracing", "uuid", + "zstd 0.13.3", ] [[package]] diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index 6a47ec78ca..b3f6ba22ce 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -160,11 +160,13 @@ fn sample_thread_with_metadata( ephemeral: bool, source: AppServerSessionSource, thread_source: Option, + parent_thread_id: Option, ) -> Thread { Thread { id: thread_id.to_string(), session_id: format!("session-{thread_id}"), forked_from_id: None, + parent_thread_id, preview: "first prompt".to_string(), ephemeral, model_provider: "openai".to_string(), @@ -195,6 +197,7 @@ fn sample_thread_start_response( ephemeral, AppServerSessionSource::Exec, Some(AppServerThreadSource::User), + /*parent_thread_id*/ None, ), model: model.to_string(), model_provider: "openai".to_string(), @@ -240,6 +243,7 @@ fn sample_thread_resume_response( model, AppServerSessionSource::Exec, Some(AppServerThreadSource::User), + /*parent_thread_id*/ None, ) } @@ -249,9 +253,16 @@ fn sample_thread_resume_response_with_source( model: &str, source: AppServerSessionSource, thread_source: Option, + parent_thread_id: Option, ) -> ClientResponsePayload { ClientResponsePayload::ThreadResume(ThreadResumeResponse { - thread: sample_thread_with_metadata(thread_id, ephemeral, source, thread_source), + thread: sample_thread_with_metadata( + thread_id, + ephemeral, + source, + thread_source, + parent_thread_id, + ), model: model.to_string(), model_provider: "openai".to_string(), service_tier: None, @@ -1755,6 +1766,7 @@ async fn compaction_event_ingests_custom_fact() { agent_role: None, }), Some(AppServerThreadSource::Subagent), + Some(parent_thread_id.to_string()), )), }, &mut events, @@ -2456,7 +2468,7 @@ fn subagent_thread_started_thread_spawn_serializes_parent_thread_id() { SubAgentThreadStartedInput { session_id: "session-root".to_string(), thread_id: "thread-spawn".to_string(), - parent_thread_id: None, + parent_thread_id: Some(parent_thread_id.to_string()), product_client_id: "codex-tui".to_string(), client_name: "codex-tui".to_string(), client_version: "1.0.0".to_string(), @@ -2534,11 +2546,14 @@ fn subagent_thread_started_other_serializes_expected_shape() { #[test] fn subagent_thread_started_other_serializes_explicit_parent_thread_id() { + let parent_thread_id = + codex_protocol::ThreadId::from_string("33333333-3333-4333-8333-333333333333") + .expect("valid thread id"); let event = TrackEventRequest::ThreadInitialized(subagent_thread_started_event_request( SubAgentThreadStartedInput { session_id: "session-root".to_string(), thread_id: "thread-guardian".to_string(), - parent_thread_id: Some("parent-thread-guardian".to_string()), + parent_thread_id: Some(parent_thread_id.to_string()), product_client_id: "codex-tui".to_string(), client_name: "codex-tui".to_string(), client_version: "1.0.0".to_string(), @@ -2553,7 +2568,7 @@ fn subagent_thread_started_other_serializes_explicit_parent_thread_id() { assert_eq!(payload["event_params"]["subagent_source"], "guardian"); assert_eq!( payload["event_params"]["parent_thread_id"], - "parent-thread-guardian" + "33333333-3333-4333-8333-333333333333" ); } @@ -2642,7 +2657,7 @@ async fn subagent_thread_started_inherits_parent_connection_for_new_thread() { SubAgentThreadStartedInput { session_id: "session-root".to_string(), thread_id: "thread-review".to_string(), - parent_thread_id: None, + parent_thread_id: Some(parent_thread_id.to_string()), product_client_id: "parent-client".to_string(), client_name: "parent-client".to_string(), client_version: "1.0.0".to_string(), diff --git a/codex-rs/analytics/src/client_tests.rs b/codex-rs/analytics/src/client_tests.rs index 4a375b0639..b7aa9f1c97 100644 --- a/codex-rs/analytics/src/client_tests.rs +++ b/codex-rs/analytics/src/client_tests.rs @@ -124,6 +124,7 @@ fn sample_thread(thread_id: &str) -> Thread { id: thread_id.to_string(), session_id: format!("session-{thread_id}"), forked_from_id: None, + parent_thread_id: None, preview: "first prompt".to_string(), ephemeral: false, model_provider: "openai".to_string(), diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 3d31be12be..fd52fefc1e 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -1012,6 +1012,7 @@ fn analytics_hook_source(source: HookSource) -> &'static str { HookSource::SessionFlags => "session_flags", HookSource::Plugin => "plugin", HookSource::CloudRequirements => "cloud_requirements", + HookSource::CloudManagedConfig => "cloud_managed_config", HookSource::LegacyManagedConfigFile => "legacy_managed_config_file", HookSource::LegacyManagedConfigMdm => "legacy_managed_config_mdm", HookSource::Unknown => "unknown", @@ -1047,9 +1048,7 @@ pub(crate) fn subagent_thread_started_event_request( thread_source: Some(ThreadSource::Subagent), initialization_mode: ThreadInitializationMode::New, subagent_source: Some(subagent_source_name(&input.subagent_source)), - parent_thread_id: input - .parent_thread_id - .or_else(|| subagent_parent_thread_id(&input.subagent_source)), + parent_thread_id: input.parent_thread_id, created_at: input.created_at, }; ThreadInitializedEvent { @@ -1059,19 +1058,7 @@ pub(crate) fn subagent_thread_started_event_request( } pub(crate) fn subagent_source_name(subagent_source: &SubAgentSource) -> String { - match subagent_source { - SubAgentSource::Review => "review".to_string(), - SubAgentSource::Compact => "compact".to_string(), - SubAgentSource::ThreadSpawn { .. } => "thread_spawn".to_string(), - SubAgentSource::MemoryConsolidation => "memory_consolidation".to_string(), - SubAgentSource::Other(other) => other.clone(), - } -} - -pub(crate) fn subagent_parent_thread_id(subagent_source: &SubAgentSource) -> Option { - subagent_source - .parent_thread_id() - .map(|parent_thread_id| parent_thread_id.to_string()) + subagent_source.kind().to_string() } fn analytics_hook_status(status: HookRunStatus) -> HookRunStatus { diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index f2f036b8f4..f206393476 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -55,7 +55,6 @@ use crate::events::codex_hook_run_metadata; use crate::events::codex_plugin_metadata; use crate::events::codex_plugin_used_metadata; use crate::events::plugin_state_event_type; -use crate::events::subagent_parent_thread_id; use crate::events::subagent_source_name; use crate::events::subagent_thread_started_event_request; use crate::facts::AnalyticsFact; @@ -267,20 +266,18 @@ impl ThreadMetadataState { session_id: String, session_source: &SessionSource, thread_source: Option, + parent_thread_id: Option, initialization_mode: ThreadInitializationMode, ) -> Self { - let (subagent_source, parent_thread_id) = match session_source { - SessionSource::SubAgent(subagent_source) => ( - Some(subagent_source_name(subagent_source)), - subagent_parent_thread_id(subagent_source), - ), + let subagent_source = match session_source { + SessionSource::SubAgent(subagent_source) => Some(subagent_source_name(subagent_source)), SessionSource::Cli | SessionSource::VSCode | SessionSource::Exec | SessionSource::Mcp | SessionSource::Custom(_) | SessionSource::Internal(_) - | SessionSource::Unknown => (None, None), + | SessionSource::Unknown => None, }; Self { session_id, @@ -516,10 +513,7 @@ impl AnalyticsReducer { input: SubAgentThreadStartedInput, out: &mut Vec, ) { - let parent_thread_id = input - .parent_thread_id - .clone() - .or_else(|| subagent_parent_thread_id(&input.subagent_source)); + let parent_thread_id = input.parent_thread_id.clone(); let parent_connection_id = parent_thread_id .as_ref() .and_then(|parent_thread_id| self.threads.get(parent_thread_id)) @@ -1238,6 +1232,7 @@ impl AnalyticsReducer { let session_source: SessionSource = thread.source.into(); let session_id = thread.session_id; let thread_id = thread.id; + let parent_thread_id = thread.parent_thread_id; let Some(connection_state) = self.connections.get(&connection_id) else { return; }; @@ -1245,6 +1240,7 @@ impl AnalyticsReducer { session_id.clone(), &session_source, thread.thread_source.map(Into::into), + parent_thread_id, initialization_mode, ); self.threads.insert( diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index b0501b0e9e..4eb5cae387 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -2002,6 +2002,7 @@ "sessionFlags", "plugin", "cloudRequirements", + "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" @@ -3365,6 +3366,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ 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 0c2ebf121e..b97c1f8fb7 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 @@ -7603,6 +7603,33 @@ "title": "SystemConfigLayerSource", "type": "object" }, + { + "description": "Enterprise-managed config layer delivered by the cloud config bundle.", + "properties": { + "id": { + "description": "Stable identifier for the delivered layer.", + "type": "string" + }, + "name": { + "description": "Admin-facing name for the delivered layer. This is surfaced in diagnostics so users know which cloud layer needs administrator attention.", + "type": "string" + }, + "type": { + "enum": [ + "enterpriseManaged" + ], + "title": "EnterpriseManagedConfigLayerSourceType", + "type": "string" + } + }, + "required": [ + "id", + "name", + "type" + ], + "title": "EnterpriseManagedConfigLayerSource", + "type": "object" + }, { "description": "User config layer from $CODEX_HOME/config.toml. This layer is special in that it is expected to be: - writable by the user - generally outside the workspace directory", "properties": { @@ -10091,6 +10118,7 @@ "sessionFlags", "plugin", "cloudRequirements", + "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" @@ -15485,6 +15513,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index c6ea51680d..f8261c86d6 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -3972,6 +3972,33 @@ "title": "SystemConfigLayerSource", "type": "object" }, + { + "description": "Enterprise-managed config layer delivered by the cloud config bundle.", + "properties": { + "id": { + "description": "Stable identifier for the delivered layer.", + "type": "string" + }, + "name": { + "description": "Admin-facing name for the delivered layer. This is surfaced in diagnostics so users know which cloud layer needs administrator attention.", + "type": "string" + }, + "type": { + "enum": [ + "enterpriseManaged" + ], + "title": "EnterpriseManagedConfigLayerSourceType", + "type": "string" + } + }, + "required": [ + "id", + "name", + "type" + ], + "title": "EnterpriseManagedConfigLayerSource", + "type": "object" + }, { "description": "User config layer from $CODEX_HOME/config.toml. This layer is special in that it is expected to be: - writable by the user - generally outside the workspace directory", "properties": { @@ -6571,6 +6598,7 @@ "sessionFlags", "plugin", "cloudRequirements", + "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" @@ -13309,6 +13337,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json index 4a104b3bd5..e714c0975d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -498,6 +498,33 @@ "title": "SystemConfigLayerSource", "type": "object" }, + { + "description": "Enterprise-managed config layer delivered by the cloud config bundle.", + "properties": { + "id": { + "description": "Stable identifier for the delivered layer.", + "type": "string" + }, + "name": { + "description": "Admin-facing name for the delivered layer. This is surfaced in diagnostics so users know which cloud layer needs administrator attention.", + "type": "string" + }, + "type": { + "enum": [ + "enterpriseManaged" + ], + "title": "EnterpriseManagedConfigLayerSourceType", + "type": "string" + } + }, + "required": [ + "id", + "name", + "type" + ], + "title": "EnterpriseManagedConfigLayerSource", + "type": "object" + }, { "description": "User config layer from $CODEX_HOME/config.toml. This layer is special in that it is expected to be: - writable by the user - generally outside the workspace directory", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigWriteResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigWriteResponse.json index 37816463b6..d1a2254a92 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigWriteResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigWriteResponse.json @@ -73,6 +73,33 @@ "title": "SystemConfigLayerSource", "type": "object" }, + { + "description": "Enterprise-managed config layer delivered by the cloud config bundle.", + "properties": { + "id": { + "description": "Stable identifier for the delivered layer.", + "type": "string" + }, + "name": { + "description": "Admin-facing name for the delivered layer. This is surfaced in diagnostics so users know which cloud layer needs administrator attention.", + "type": "string" + }, + "type": { + "enum": [ + "enterpriseManaged" + ], + "title": "EnterpriseManagedConfigLayerSourceType", + "type": "string" + } + }, + "required": [ + "id", + "name", + "type" + ], + "title": "EnterpriseManagedConfigLayerSource", + "type": "object" + }, { "description": "User config layer from $CODEX_HOME/config.toml. This layer is special in that it is expected to be: - writable by the user - generally outside the workspace directory", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json index 088da47484..8684bf9ae5 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json @@ -166,6 +166,7 @@ "sessionFlags", "plugin", "cloudRequirements", + "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json index 38395cded9..5b2750d780 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json @@ -166,6 +166,7 @@ "sessionFlags", "plugin", "cloudRequirements", + "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json index c58a5c767d..c3288ee0fd 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json @@ -130,6 +130,7 @@ "sessionFlags", "plugin", "cloudRequirements", + "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", "unknown" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index ec46130357..0d3a3fe8db 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -1036,6 +1036,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json index 5ace3f7af7..a500c21587 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json @@ -851,6 +851,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json index a4cc7d91ff..6e08edb962 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json @@ -851,6 +851,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json index 234e0867f8..eed031d1e4 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json @@ -851,6 +851,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index 59ed236c48..3a9e51f2a7 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -1036,6 +1036,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json index 7815f261c0..9bc687fbfb 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json @@ -851,6 +851,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index ca52f10ca5..8640c4e9ed 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -1036,6 +1036,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json index f6cb72b6b2..f7830e221e 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json @@ -851,6 +851,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json index e4e317e608..9a634c1110 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json @@ -851,6 +851,13 @@ "null" ] }, + "parentThreadId": { + "description": "The ID of the parent thread. This will only be set if this thread is a subagent.", + "type": [ + "string", + "null" + ] + }, "path": { "description": "[UNSTABLE] Path to the thread on disk.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigLayerSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigLayerSource.ts index 08cb8c6bfd..963c331b5d 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ConfigLayerSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ConfigLayerSource.ts @@ -8,7 +8,17 @@ export type ConfigLayerSource = { "type": "mdm", domain: string, key: string, } * This is the path to the system config.toml file, though it is not * guaranteed to exist. */ -file: AbsolutePathBuf, } | { "type": "user", +file: AbsolutePathBuf, } | { "type": "enterpriseManaged", +/** + * Stable identifier for the delivered layer. + */ +id: string, +/** + * Admin-facing name for the delivered layer. This is surfaced in + * diagnostics so users know which cloud layer needs administrator + * attention. + */ +name: string, } | { "type": "user", /** * This is the path to the user's config.toml file, though it is not * guaranteed to exist. diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts index 98bbe1e412..5c3cb2eabf 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type HookSource = "system" | "user" | "project" | "mdm" | "sessionFlags" | "plugin" | "cloudRequirements" | "legacyManagedConfigFile" | "legacyManagedConfigMdm" | "unknown"; +export type HookSource = "system" | "user" | "project" | "mdm" | "sessionFlags" | "plugin" | "cloudRequirements" | "cloudManagedConfig" | "legacyManagedConfigFile" | "legacyManagedConfigMdm" | "unknown"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts b/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts index d917094e36..5fa30b64e3 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/Thread.ts @@ -17,6 +17,10 @@ sessionId: string, * Source thread id when this thread was created by forking another thread. */ forkedFromId: string | null, +/** + * The ID of the parent thread. This will only be set if this thread is a subagent. + */ +parentThreadId: string | null, /** * Usually the first user message in the thread, if available. */ diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 71c89a185c..d532952f6c 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -2328,6 +2328,7 @@ mod tests { id: "67e55044-10b1-426f-9247-bb680e5fe0c8".to_string(), session_id: "67e55044-10b1-426f-9247-bb680e5fe0c7".to_string(), forked_from_id: None, + parent_thread_id: None, preview: "first prompt".to_string(), ephemeral: true, model_provider: "openai".to_string(), @@ -2370,6 +2371,7 @@ mod tests { "id": "67e55044-10b1-426f-9247-bb680e5fe0c8", "sessionId": "67e55044-10b1-426f-9247-bb680e5fe0c7", "forkedFromId": null, + "parentThreadId": null, "preview": "first prompt", "ephemeral": true, "modelProvider": "openai", diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index 05a31cf44d..473bb1e03c 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -43,6 +43,19 @@ pub enum ConfigLayerSource { file: AbsolutePathBuf, }, + /// Enterprise-managed config layer delivered by the cloud config bundle. + #[serde(rename_all = "camelCase")] + #[ts(rename_all = "camelCase")] + EnterpriseManaged { + /// Stable identifier for the delivered layer. + id: String, + + /// Admin-facing name for the delivered layer. This is surfaced in + /// diagnostics so users know which cloud layer needs administrator + /// attention. + name: String, + }, + /// User config layer from $CODEX_HOME/config.toml. This layer is special /// in that it is expected to be: /// - writable by the user @@ -90,6 +103,7 @@ impl ConfigLayerSource { match self { ConfigLayerSource::Mdm { .. } => 0, ConfigLayerSource::System { .. } => 10, + ConfigLayerSource::EnterpriseManaged { .. } => 15, ConfigLayerSource::User { profile, .. } => { if profile.is_some() { 21 diff --git a/codex-rs/app-server-protocol/src/protocol/v2/hook.rs b/codex-rs/app-server-protocol/src/protocol/v2/hook.rs index 6e6c1ef7d4..b8f47276a6 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/hook.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/hook.rs @@ -48,6 +48,7 @@ v2_enum_from_core!( SessionFlags, Plugin, CloudRequirements, + CloudManagedConfig, LegacyManagedConfigFile, LegacyManagedConfigMdm, Unknown, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index bb6003c7ae..1e7558363d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -142,6 +142,7 @@ fn thread_resume_response_round_trips_initial_turns_page() { id: "thr_123".to_string(), session_id: "thr_123".to_string(), forked_from_id: None, + parent_thread_id: None, preview: String::new(), ephemeral: false, model_provider: "openai".to_string(), @@ -3581,6 +3582,7 @@ fn thread_lifecycle_responses_default_missing_optional_fields() { let fork: ThreadForkResponse = serde_json::from_value(response).expect("thread/fork response"); assert_eq!(start.instruction_sources, Vec::::new()); + assert_eq!(start.thread.parent_thread_id, None); assert_eq!(resume.instruction_sources, Vec::::new()); assert_eq!(fork.instruction_sources, Vec::::new()); assert_eq!(start.active_permission_profile, None); diff --git a/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs b/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs index f0c518adf8..35b6183871 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs @@ -108,6 +108,8 @@ pub struct Thread { pub session_id: String, /// Source thread id when this thread was created by forking another thread. pub forked_from_id: Option, + /// The ID of the parent thread. This will only be set if this thread is a subagent. + pub parent_thread_id: Option, /// Usually the first user message in the thread, if available. pub preview: String, /// Whether the thread is ephemeral and should not be materialized on disk. diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2bf6035b27..29acc51983 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -134,7 +134,7 @@ Example with notification opt-out: - `thread/resume` — reopen an existing thread by id so subsequent `turn/start` calls append to it. Accepts the same permission override rules as `thread/start`. - `thread/fork` — fork an existing thread into a new thread id by copying the stored history; if the source thread is currently mid-turn, the fork records the same interruption marker as `turn/interrupt` instead of inheriting an unmarked partial turn suffix. The returned `thread.forkedFromId` points at the source thread when known. Accepts `ephemeral: true` for an in-memory temporary fork, emits `thread/started` (including the current `thread.status`), and auto-subscribes you to turn/item events for the new thread. Experimental clients can pass `excludeTurns: true` when they plan to page fork history via `thread/turns/list` instead of receiving the full turn array immediately. Accepts the same permission override rules as `thread/start`. - `thread/start`, `thread/resume`, and `thread/fork` responses include the legacy `sandbox` compatibility projection. Experimental clients can read `runtimeWorkspaceRoots` for the thread-scoped runtime roots and `activePermissionProfile` for the named or implicit built-in profile identity/provenance when known. -- `thread/list` — page through stored rollouts; supports cursor-based pagination and optional `modelProviders`, `sourceKinds`, `archived`, `cwd`, and `searchTerm` filters. Each returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. +- `thread/list` — page through stored rollouts; supports cursor-based pagination and optional `modelProviders`, `sourceKinds`, `archived`, `cwd`, and `searchTerm` filters. Each returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. Subagent threads also include `parentThreadId` when the immediate control/spawn parent is known. - `thread/loaded/list` — list the thread ids currently loaded in memory. - `thread/read` — read a stored thread by id without resuming it; optionally include turns via `includeTurns`. The returned `thread` includes `status` (`ThreadStatus`), defaulting to `notLoaded` when the thread is not currently loaded. - `thread/turns/list` — experimental; page through a stored thread’s turn history without resuming it; supports cursor-based pagination with `sortDirection`, `itemsView`, `nextCursor`, and `backwardsCursor`. @@ -424,7 +424,7 @@ Later, after the idle unload timeout: ### Example: Read a thread -Use `thread/read` to fetch a stored thread by id without resuming it. Pass `includeTurns` when you want thread history loaded into `thread.turns`. The returned thread includes `agentNickname` and `agentRole` for AgentControl-spawned thread sub-agents when available. +Use `thread/read` to fetch a stored thread by id without resuming it. Pass `includeTurns` when you want thread history loaded into `thread.turns`. The returned thread includes `parentThreadId`, `agentNickname`, and `agentRole` for subagent threads when available. ```json { "method": "thread/read", "id": 22, "params": { "threadId": "thr_123" } } diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index b870b5b17c..56754868fc 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -2170,6 +2170,7 @@ mod tests { thread_id, rollout_path: None, forked_from_id: None, + parent_thread_id: None, preview: "fallback preview".to_string(), name: Some("Rollback thread".to_string()), model_provider: "openai".to_string(), diff --git a/codex-rs/app-server/src/config_manager_service.rs b/codex-rs/app-server/src/config_manager_service.rs index d2465d32dd..4b42c28aa9 100644 --- a/codex-rs/app-server/src/config_manager_service.rs +++ b/codex-rs/app-server/src/config_manager_service.rs @@ -624,6 +624,9 @@ fn override_message(layer: &ConfigLayerSource) -> String { ConfigLayerSource::System { file } => { format!("Overridden by managed config (system): {}", file.display()) } + ConfigLayerSource::EnterpriseManaged { id: _, name } => { + format!("Overridden by enterprise-managed config: {name}") + } ConfigLayerSource::Project { dot_codex_folder } => format!( "Overridden by project config: {}/{CONFIG_TOML_FILE}", dot_codex_folder.display(), diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index cce6c6b2cb..0b39d8955e 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -4015,6 +4015,7 @@ pub(crate) fn thread_from_stored_thread( id: thread_id.clone(), session_id: thread_id, forked_from_id: thread.forked_from_id.map(|id| id.to_string()), + parent_thread_id: thread.parent_thread_id.map(|id| id.to_string()), preview: thread.preview, ephemeral: false, model_provider: if thread.model_provider.is_empty() { @@ -4223,6 +4224,7 @@ fn build_thread_from_snapshot( id: thread_id.to_string(), session_id, forked_from_id: None, + parent_thread_id: config_snapshot.parent_thread_id.map(|id| id.to_string()), preview: String::new(), ephemeral: config_snapshot.ephemeral, model_provider: config_snapshot.model_provider_id.clone(), diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index 2a4b2722ec..00fb3fff4e 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -395,6 +395,7 @@ mod thread_processor_behavior_tests { thread_id, rollout_path: Some(PathBuf::from("/tmp/thread.jsonl")), forked_from_id: None, + parent_thread_id: None, preview: "preview".to_string(), name: None, model_provider: "openai".to_string(), @@ -681,6 +682,7 @@ mod thread_processor_behavior_tests { }, }, session_source: SessionSource::Cli, + parent_thread_id: None, thread_source: None, }; diff --git a/codex-rs/app-server/src/request_processors/thread_resume_redaction.rs b/codex-rs/app-server/src/request_processors/thread_resume_redaction.rs index b265e3a632..e970a88a9f 100644 --- a/codex-rs/app-server/src/request_processors/thread_resume_redaction.rs +++ b/codex-rs/app-server/src/request_processors/thread_resume_redaction.rs @@ -172,6 +172,7 @@ mod tests { id: "thread-1".to_string(), session_id: "session-1".to_string(), forked_from_id: None, + parent_thread_id: None, preview: "preview".to_string(), ephemeral: false, model_provider: "mock_provider".to_string(), diff --git a/codex-rs/app-server/src/request_processors/thread_summary.rs b/codex-rs/app-server/src/request_processors/thread_summary.rs index 0a6c1bfb0f..da7ae54804 100644 --- a/codex-rs/app-server/src/request_processors/thread_summary.rs +++ b/codex-rs/app-server/src/request_processors/thread_summary.rs @@ -305,6 +305,7 @@ pub(crate) fn summary_to_thread( id: thread_id.clone(), session_id: thread_id, forked_from_id: None, + parent_thread_id: None, preview, ephemeral: false, model_provider, diff --git a/codex-rs/app-server/src/thread_status.rs b/codex-rs/app-server/src/thread_status.rs index 7315a13c02..6d66a32d6a 100644 --- a/codex-rs/app-server/src/thread_status.rs +++ b/codex-rs/app-server/src/thread_status.rs @@ -891,6 +891,7 @@ mod tests { id: thread_id.to_string(), session_id: thread_id.to_string(), forked_from_id: None, + parent_thread_id: None, preview: String::new(), ephemeral: false, model_provider: "mock-provider".to_string(), diff --git a/codex-rs/app-server/tests/common/lib.rs b/codex-rs/app-server/tests/common/lib.rs index 6bb600bd82..46eeae7ec1 100644 --- a/codex-rs/app-server/tests/common/lib.rs +++ b/codex-rs/app-server/tests/common/lib.rs @@ -38,6 +38,7 @@ pub use responses::create_final_assistant_message_sse_response; pub use responses::create_request_permissions_sse_response; pub use responses::create_request_user_input_sse_response; pub use responses::create_shell_command_sse_response; +pub use rollout::create_fake_parented_rollout_with_source; pub use rollout::create_fake_rollout; pub use rollout::create_fake_rollout_with_source; pub use rollout::create_fake_rollout_with_text_elements; diff --git a/codex-rs/app-server/tests/common/rollout.rs b/codex-rs/app-server/tests/common/rollout.rs index 6b2a9a0abe..63681382ea 100644 --- a/codex-rs/app-server/tests/common/rollout.rs +++ b/codex-rs/app-server/tests/common/rollout.rs @@ -118,6 +118,53 @@ pub fn create_fake_rollout_with_source( model_provider: Option<&str>, git_info: Option, source: SessionSource, +) -> Result { + create_fake_rollout_with_source_and_parent_thread_id( + codex_home, + filename_ts, + meta_rfc3339, + preview, + model_provider, + git_info, + source, + /*parent_thread_id*/ None, + ) +} + +/// Create a minimal rollout file with an explicit session source and control parent. +#[allow(clippy::too_many_arguments)] +pub fn create_fake_parented_rollout_with_source( + codex_home: &Path, + filename_ts: &str, + meta_rfc3339: &str, + preview: &str, + model_provider: Option<&str>, + git_info: Option, + source: SessionSource, + parent_thread_id: ThreadId, +) -> Result { + create_fake_rollout_with_source_and_parent_thread_id( + codex_home, + filename_ts, + meta_rfc3339, + preview, + model_provider, + git_info, + source, + Some(parent_thread_id), + ) +} + +#[allow(clippy::too_many_arguments)] +fn create_fake_rollout_with_source_and_parent_thread_id( + codex_home: &Path, + filename_ts: &str, + meta_rfc3339: &str, + preview: &str, + model_provider: Option<&str>, + git_info: Option, + source: SessionSource, + parent_thread_id: Option, ) -> Result { let uuid = Uuid::new_v4(); let uuid_str = uuid.to_string(); @@ -133,6 +180,7 @@ pub fn create_fake_rollout_with_source( let meta = SessionMeta { id: conversation_id, forked_from_id: None, + parent_thread_id, timestamp: meta_rfc3339.to_string(), cwd: PathBuf::from("/"), originator: "codex".to_string(), @@ -217,6 +265,7 @@ pub fn create_fake_rollout_with_text_elements( let meta = SessionMeta { id: conversation_id, forked_from_id: None, + parent_thread_id: None, timestamp: meta_rfc3339.to_string(), cwd: PathBuf::from("/"), originator: "codex".to_string(), diff --git a/codex-rs/app-server/tests/suite/conversation_summary.rs b/codex-rs/app-server/tests/suite/conversation_summary.rs index 5253850107..eb534e517f 100644 --- a/codex-rs/app-server/tests/suite/conversation_summary.rs +++ b/codex-rs/app-server/tests/suite/conversation_summary.rs @@ -124,6 +124,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() -> .create_thread(CreateThreadParams { thread_id, forked_from_id: None, + parent_thread_id: None, source: SessionSource::Cli, thread_source: None, base_instructions: BaseInstructions::default(), diff --git a/codex-rs/app-server/tests/suite/v2/client_metadata.rs b/codex-rs/app-server/tests/suite/v2/client_metadata.rs index ab94fe618e..2e6ede73d7 100644 --- a/codex-rs/app-server/tests/suite/v2/client_metadata.rs +++ b/codex-rs/app-server/tests/suite/v2/client_metadata.rs @@ -1,7 +1,7 @@ use anyhow::Result; use app_test_support::McpProcess; +use app_test_support::create_fake_parented_rollout_with_source; use app_test_support::create_fake_rollout; -use app_test_support::create_fake_rollout_with_source; use app_test_support::to_response; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; @@ -9,6 +9,7 @@ use codex_app_server_protocol::ReviewDelivery; use codex_app_server_protocol::ReviewStartParams; use codex_app_server_protocol::ReviewStartResponse; use codex_app_server_protocol::ReviewTarget; +use codex_app_server_protocol::SessionSource as ApiSessionSource; use codex_app_server_protocol::ThreadForkParams; use codex_app_server_protocol::ThreadForkResponse; use codex_app_server_protocol::ThreadResumeParams; @@ -198,7 +199,7 @@ async fn turn_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() -> } #[tokio::test] -async fn review_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() -> Result<()> { +async fn review_start_sends_parent_lineage_in_turn_metadata_for_thread_fork_v2() -> Result<()> { skip_if_no_network!(Ok(())); let review_payload = serde_json::json!({ @@ -276,8 +277,9 @@ async fn review_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() - request.header("x-openai-subagent").as_deref(), Some("review") ); + assert!(metadata.get("forked_from_thread_id").is_none()); assert_eq!( - metadata["forked_from_thread_id"].as_str(), + metadata["parent_thread_id"].as_str(), Some(review_thread_id.as_str()) ); let review_request_thread_id = metadata["thread_id"] @@ -297,7 +299,7 @@ async fn review_start_sends_fork_lineage_in_turn_metadata_for_thread_fork_v2() - } #[tokio::test] -async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Result<()> { +async fn turn_start_sends_other_subagent_lineage_after_cold_thread_resume_v2() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; @@ -320,20 +322,15 @@ async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Resu let parent_thread_id = CoreThreadId::new(); let parent_thread_id_str = parent_thread_id.to_string(); - let subagent_thread_id = create_fake_rollout_with_source( + let subagent_thread_id = create_fake_parented_rollout_with_source( codex_home.path(), "2025-01-05T12-00-00", "2025-01-05T12:00:00Z", "Saved subagent message", Some("mock_provider"), /*git_info*/ None, - SessionSource::SubAgent(SubAgentSource::ThreadSpawn { - parent_thread_id, - depth: 1, - agent_path: None, - agent_nickname: None, - agent_role: None, - }), + SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())), + parent_thread_id, )?; let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -352,6 +349,11 @@ async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Resu .await??; let ThreadResumeResponse { thread, .. } = to_response::(resume_resp)?; assert_eq!(thread.id, subagent_thread_id); + assert_eq!(thread.parent_thread_id, Some(parent_thread_id_str.clone())); + assert_eq!( + thread.source, + ApiSessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())) + ); let turn_req = mcp .send_turn_start_request(TurnStartParams { @@ -386,7 +388,7 @@ async fn turn_start_sends_subagent_lineage_after_cold_thread_resume_v2() -> Resu metadata["parent_thread_id"].as_str(), Some(parent_thread_id_str.as_str()) ); - assert_eq!(metadata["subagent_kind"].as_str(), Some("thread_spawn")); + assert_eq!(metadata["subagent_kind"].as_str(), Some("guardian")); assert_eq!(metadata["thread_id"].as_str(), Some(thread.id.as_str())); assert_eq!(metadata["turn_id"].as_str(), Some(turn.id.as_str())); assert!(metadata.get("forked_from_thread_id").is_none()); diff --git a/codex-rs/app-server/tests/suite/v2/thread_list.rs b/codex-rs/app-server/tests/suite/v2/thread_list.rs index 8808092361..77e2e281f9 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_list.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_list.rs @@ -1,5 +1,6 @@ use anyhow::Result; use app_test_support::McpProcess; +use app_test_support::create_fake_parented_rollout_with_source; use app_test_support::create_fake_rollout; use app_test_support::create_fake_rollout_with_source; use app_test_support::create_final_assistant_message_sse_response; @@ -1049,7 +1050,7 @@ async fn thread_list_filters_by_subagent_variant() -> Result<()> { let parent_thread_id = ThreadId::from_string(&Uuid::new_v4().to_string())?; - let review_id = create_fake_rollout_with_source( + let review_id = create_fake_parented_rollout_with_source( codex_home.path(), "2025-02-02T09-00-00", "2025-02-02T09:00:00Z", @@ -1057,6 +1058,7 @@ async fn thread_list_filters_by_subagent_variant() -> Result<()> { Some("mock_provider"), /*git_info*/ None, CoreSessionSource::SubAgent(SubAgentSource::Review), + parent_thread_id, )?; let compact_id = create_fake_rollout_with_source( codex_home.path(), @@ -1109,6 +1111,10 @@ async fn thread_list_filters_by_subagent_variant() -> Result<()> { .map(|thread| thread.id.as_str()) .collect(); assert_eq!(review_ids, vec![review_id.as_str()]); + assert_eq!( + review.data[0].parent_thread_id, + Some(parent_thread_id.to_string()) + ); let compact = list_threads( &mut mcp, diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index ee430f1593..df79aea9b4 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -1358,6 +1358,7 @@ async fn seed_pathless_store_thread( .create_thread(CreateThreadParams { thread_id, forked_from_id: None, + parent_thread_id: None, source: ProtocolSessionSource::Cli, thread_source: None, base_instructions: BaseInstructions::default(), 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 8b302954e0..e0263d77f3 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -1791,6 +1791,7 @@ stream_max_retries = 0 let session_meta = SessionMeta { id: conversation_id, forked_from_id: None, + parent_thread_id: None, timestamp: "2025-01-05T12:00:00Z".to_string(), cwd: repo_path.clone(), originator: "codex".to_string(), diff --git a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs index dea8433d44..46ce38f48d 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs @@ -211,6 +211,7 @@ async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> { .create_thread(CreateThreadParams { thread_id, forked_from_id: Some(parent_thread_id), + parent_thread_id: None, source: SessionSource::Cli, thread_source: None, base_instructions: BaseInstructions::default(), diff --git a/codex-rs/config/src/cloud_config_layers.rs b/codex-rs/config/src/cloud_config_layers.rs new file mode 100644 index 0000000000..bdb11631e4 --- /dev/null +++ b/codex-rs/config/src/cloud_config_layers.rs @@ -0,0 +1,108 @@ +//! Conversion from cloud-delivered config TOML fragments into config stack layers. +//! +//! Backend fragments arrive in backend priority order. This module parses each +//! fragment, resolves relative path fields against the cloud config base +//! directory, and returns layers in `ConfigLayerStack` order. + +use crate::ConfigLayerEntry; +use crate::ConfigLayerSource; +use crate::TomlValue; +use crate::loader::resolve_relative_paths_in_config_toml; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::fmt; +use std::io; +use thiserror::Error; + +/// Config fragment delivered by the cloud config bundle. +/// +/// The bundle orders fragments from highest precedence to lowest precedence. +/// This module returns config layers in stack order, so callers can append the +/// result between system and user config without re-sorting. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct CloudConfigFragment { + pub id: String, + pub name: String, + pub contents: String, +} + +impl CloudConfigFragment { + fn source_ref(&self) -> CloudConfigFragmentSource { + CloudConfigFragmentSource { + id: self.id.clone(), + name: self.name.clone(), + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct CloudConfigFragmentSource { + pub id: String, + pub name: String, +} + +impl fmt::Display for CloudConfigFragmentSource { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} ({})", self.name, self.id) + } +} + +#[derive(Debug, Error, PartialEq, Eq)] +pub enum CloudConfigLayerError { + #[error("failed to parse cloud config fragment {fragment}: {message}")] + Parse { + fragment: CloudConfigFragmentSource, + message: String, + }, + #[error("invalid cloud config fragment {fragment}: {message}")] + Invalid { + fragment: CloudConfigFragmentSource, + message: String, + }, +} + +pub fn cloud_config_layers_from_fragments( + fragments: impl IntoIterator, + base_dir: &AbsolutePathBuf, +) -> Result, CloudConfigLayerError> { + let mut layers = Vec::new(); + for fragment in fragments { + let source_ref = fragment.source_ref(); + let raw_toml = fragment.contents; + let value: TomlValue = + toml::from_str(&raw_toml).map_err(|err| CloudConfigLayerError::Parse { + fragment: source_ref.clone(), + message: err.to_string(), + })?; + let resolved = + resolve_relative_paths_in_config_toml(value, base_dir.as_path()).map_err(|err| { + CloudConfigLayerError::Invalid { + fragment: source_ref.clone(), + message: err.to_string(), + } + })?; + layers.push(ConfigLayerEntry::new_with_raw_toml( + ConfigLayerSource::EnterpriseManaged { + id: fragment.id, + name: fragment.name, + }, + resolved, + raw_toml, + base_dir.clone(), + )); + } + + // Bundle fragments arrive highest-priority first, while ConfigLayerStack + // folds lowest-priority to highest-priority. + layers.reverse(); + Ok(layers) +} + +impl From for io::Error { + fn from(error: CloudConfigLayerError) -> Self { + io::Error::new(io::ErrorKind::InvalidData, error) + } +} + +#[cfg(test)] +#[path = "cloud_config_layers_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/cloud_config_layers_tests.rs b/codex-rs/config/src/cloud_config_layers_tests.rs new file mode 100644 index 0000000000..c7f685fa67 --- /dev/null +++ b/codex-rs/config/src/cloud_config_layers_tests.rs @@ -0,0 +1,210 @@ +use super::*; +use crate::CONFIG_TOML_FILE; +use crate::ConfigLayerStack; +use crate::ConfigLayerStackOrdering; +use crate::ConfigRequirements; +use crate::ConfigRequirementsToml; +use crate::config_toml::ConfigToml; +use crate::first_layer_config_error_from_entries; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::test_support::PathBufExt; +use codex_utils_absolute_path::test_support::test_path_buf; +use pretty_assertions::assert_eq; +use std::path::Path; + +fn fragment(id: &str, name: &str, contents: &str) -> CloudConfigFragment { + CloudConfigFragment { + id: id.to_string(), + name: name.to_string(), + contents: contents.to_string(), + } +} + +fn toml(contents: &str) -> TomlValue { + toml::from_str(contents).expect("test TOML should parse") +} + +fn base_dir() -> AbsolutePathBuf { + test_path_buf("/var/lib/codex").abs() +} + +#[test] +fn layers_are_returned_in_stack_order() { + let base_dir = base_dir(); + let layers = cloud_config_layers_from_fragments( + vec![ + fragment("high", "High priority", "model = \"cloud-high\""), + fragment("low", "Low priority", "model_provider = \"cloud-low\""), + ], + &base_dir, + ) + .expect("cloud config layers should compose"); + + assert_eq!( + layers + .iter() + .map(|layer| layer.name.clone()) + .collect::>(), + vec![ + ConfigLayerSource::EnterpriseManaged { + id: "low".to_string(), + name: "Low priority".to_string(), + }, + ConfigLayerSource::EnterpriseManaged { + id: "high".to_string(), + name: "High priority".to_string(), + }, + ] + ); +} + +#[test] +fn enterprise_layers_precede_user_and_override_system() { + let base_dir = base_dir(); + let mut layers = vec![ConfigLayerEntry::new( + ConfigLayerSource::System { + file: test_path_buf("/etc/codex/config.toml").abs(), + }, + toml( + r#" +model = "system" +model_provider = "system" +review_model = "system-review" +"#, + ), + )]; + layers.extend( + cloud_config_layers_from_fragments( + vec![ + fragment("high", "High priority", "model_provider = \"cloud-high\""), + fragment("low", "Low priority", "review_model = \"cloud-low-review\""), + ], + &base_dir, + ) + .expect("cloud config layers should compose"), + ); + layers.push(ConfigLayerEntry::new( + ConfigLayerSource::User { + file: test_path_buf("/home/alice/.codex/config.toml").abs(), + profile: None, + }, + toml("model = \"user\""), + )); + + let stack = ConfigLayerStack::new( + layers, + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("stack should be ordered"); + + assert_eq!( + stack + .get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ false + ) + .iter() + .map(|layer| layer.name.clone()) + .collect::>(), + vec![ + ConfigLayerSource::System { + file: test_path_buf("/etc/codex/config.toml").abs(), + }, + ConfigLayerSource::EnterpriseManaged { + id: "low".to_string(), + name: "Low priority".to_string(), + }, + ConfigLayerSource::EnterpriseManaged { + id: "high".to_string(), + name: "High priority".to_string(), + }, + ConfigLayerSource::User { + file: test_path_buf("/home/alice/.codex/config.toml").abs(), + profile: None, + }, + ] + ); + assert_eq!( + stack.effective_config(), + toml( + r#" +model = "user" +model_provider = "cloud-high" +review_model = "cloud-low-review" +"#, + ) + ); +} + +#[test] +fn relative_absolute_path_fields_resolve_against_base_dir() { + let base_dir = base_dir(); + let layers = cloud_config_layers_from_fragments( + vec![fragment( + "cfg_123", + "Base policy", + "model_instructions_file = \"instructions.md\"", + )], + &base_dir, + ) + .expect("relative paths should match existing MDM semantics"); + + let path = layers[0] + .config + .get("model_instructions_file") + .and_then(TomlValue::as_str) + .expect("path should be present"); + let expected = + AbsolutePathBuf::resolve_path_against_base("instructions.md", base_dir.as_path()); + assert_eq!(path, expected.to_string_lossy()); +} + +#[test] +fn home_relative_path_fields_are_allowed_and_resolved() { + let base_dir = base_dir(); + let layers = cloud_config_layers_from_fragments( + vec![fragment( + "cfg_123", + "Base policy", + "model_instructions_file = \"~/instructions.md\"", + )], + &base_dir, + ) + .expect("home-relative paths should be accepted"); + + let path = layers[0] + .config + .get("model_instructions_file") + .and_then(TomlValue::as_str) + .expect("path should be present"); + let expected = + AbsolutePathBuf::resolve_path_against_base("~/instructions.md", base_dir.as_path()); + assert_eq!(path, expected.to_string_lossy()); +} + +#[tokio::test] +async fn raw_toml_diagnostics_use_enterprise_layer_name() { + let base_dir = base_dir(); + let layers = cloud_config_layers_from_fragments( + vec![fragment( + "cfg_123", + "Base policy", + "model_instructions_file = \"instructions.md\"\nmodel = 1", + )], + &base_dir, + ) + .expect("cloud config layers should parse"); + + let error = first_layer_config_error_from_entries::(&layers, CONFIG_TOML_FILE) + .await + .expect("invalid raw TOML should produce a layer diagnostic"); + + assert_eq!( + error.path, + Path::new("enterprise-managed (Base policy, cfg_123)").to_path_buf() + ); + assert_eq!(error.range.start.line, 2); + assert_eq!(error.range.start.column, 9); + assert!(error.message.contains("invalid type: integer `1`")); +} diff --git a/codex-rs/config/src/config_layer_source.rs b/codex-rs/config/src/config_layer_source.rs new file mode 100644 index 0000000000..7bff779b19 --- /dev/null +++ b/codex-rs/config/src/config_layer_source.rs @@ -0,0 +1,31 @@ +use codex_app_server_protocol::ConfigLayerSource; + +pub fn format_config_layer_source(source: &ConfigLayerSource, config_toml_file: &str) -> String { + match source { + ConfigLayerSource::Mdm { domain, key } => { + format!("MDM ({domain}:{key})") + } + ConfigLayerSource::System { file } => { + format!("system ({})", file.as_path().display()) + } + ConfigLayerSource::EnterpriseManaged { id, name } => { + format!("enterprise-managed ({name}, {id})") + } + ConfigLayerSource::User { file, .. } => { + format!("user ({})", file.as_path().display()) + } + ConfigLayerSource::Project { dot_codex_folder } => { + format!( + "project ({}/{config_toml_file})", + dot_codex_folder.as_path().display() + ) + } + ConfigLayerSource::SessionFlags => "session-flags".to_string(), + ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => { + format!("legacy managed_config.toml ({})", file.as_path().display()) + } + ConfigLayerSource::LegacyManagedConfigTomlFromMdm => { + "legacy managed_config.toml (MDM)".to_string() + } + } +} diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index 1f92591bc5..86d64e4153 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -25,13 +25,61 @@ use crate::types::WindowsSandboxModeToml; #[derive(Debug, Clone, PartialEq, Eq)] pub enum RequirementSource { Unknown, - MdmManagedPreferences { domain: String, key: String }, + MdmManagedPreferences { + domain: String, + key: String, + }, CloudRequirements, - SystemRequirementsToml { file: AbsolutePathBuf }, - LegacyManagedConfigTomlFromFile { file: AbsolutePathBuf }, + /// Multiple requirements layers contributed to the final value. Sources are + /// stored highest-priority first, matching the order surfaced in errors. + Composite { + sources: Vec, + }, + /// A backend-delivered enterprise-managed layer. `id` is the stable backend + /// identifier; `name` is the admin-facing display name. + EnterpriseManaged { + id: String, + name: String, + }, + SystemRequirementsToml { + file: AbsolutePathBuf, + }, + LegacyManagedConfigTomlFromFile { + file: AbsolutePathBuf, + }, LegacyManagedConfigTomlFromMdm, } +impl RequirementSource { + pub fn composite(sources: impl IntoIterator) -> Self { + let mut flattened = Vec::new(); + for source in sources { + source.append_to_composite(&mut flattened); + } + + match flattened.len() { + 0 => RequirementSource::Unknown, + 1 => flattened.remove(0), + _ => RequirementSource::Composite { sources: flattened }, + } + } + + fn append_to_composite(self, flattened: &mut Vec) { + match self { + RequirementSource::Composite { sources } => { + for source in sources { + source.append_to_composite(flattened); + } + } + source => { + if !flattened.contains(&source) { + flattened.push(source); + } + } + } + } +} + impl fmt::Display for RequirementSource { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -42,6 +90,19 @@ impl fmt::Display for RequirementSource { RequirementSource::CloudRequirements => { write!(f, "cloud requirements") } + RequirementSource::Composite { sources } => { + write!(f, "requirements layers: ")?; + for (index, source) in sources.iter().enumerate() { + if index > 0 { + write!(f, ", ")?; + } + write!(f, "{source}")?; + } + Ok(()) + } + RequirementSource::EnterpriseManaged { id, name } => { + write!(f, "enterprise-managed requirements {name} ({id})") + } RequirementSource::SystemRequirementsToml { file } => { write!(f, "{}", file.as_path().display()) } @@ -983,7 +1044,7 @@ fn hostname_matches_any_pattern(hostname: &str, patterns: &[String]) -> bool { /// Currently, `external-sandbox` is not supported in config.toml, but it is /// supported through programmatic use. -#[derive(Deserialize, Debug, Clone, Copy, PartialEq)] +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq)] pub enum SandboxModeRequirement { #[serde(rename = "read-only")] ReadOnly, @@ -1430,6 +1491,41 @@ mod tests { )?) } + #[test] + fn composite_requirement_source_flattens_and_deduplicates_sources() { + let mdm_source = RequirementSource::MdmManagedPreferences { + domain: "com.openai.codex".to_string(), + key: "requirements_toml_base64".to_string(), + }; + let legacy_source = RequirementSource::LegacyManagedConfigTomlFromMdm; + + assert_eq!( + RequirementSource::composite([ + mdm_source.clone(), + RequirementSource::composite([legacy_source.clone(), mdm_source.clone()]), + ]), + RequirementSource::Composite { + sources: vec![mdm_source, legacy_source], + } + ); + } + + #[test] + fn composite_requirement_source_display_lists_sources_in_priority_order() { + let source = RequirementSource::composite([ + RequirementSource::MdmManagedPreferences { + domain: "com.openai.codex".to_string(), + key: "requirements_toml_base64".to_string(), + }, + RequirementSource::LegacyManagedConfigTomlFromMdm, + ]); + + assert_eq!( + source.to_string(), + "requirements layers: MDM com.openai.codex:requirements_toml_base64, MDM managed_config.toml (legacy)" + ); + } + fn with_unknown_source(toml: ConfigRequirementsToml) -> ConfigRequirementsWithSources { let ConfigRequirementsToml { allowed_approval_policies, @@ -2273,14 +2369,20 @@ allowed_approvals_reviewers = ["user"] } #[test] - fn constraint_error_includes_cloud_requirements_source() -> Result<()> { + fn constraint_error_includes_composite_requirement_source() -> Result<()> { let source: ConfigRequirementsToml = from_str( r#" allowed_approval_policies = ["on-request"] "#, )?; - let source_location = RequirementSource::CloudRequirements; + let source_location = RequirementSource::composite([ + RequirementSource::MdmManagedPreferences { + domain: "com.openai.codex".to_string(), + key: "requirements_toml_base64".to_string(), + }, + RequirementSource::LegacyManagedConfigTomlFromMdm, + ]); let mut target = ConfigRequirementsWithSources::default(); target.merge_unset_fields(source_location.clone(), source); diff --git a/codex-rs/config/src/diagnostics.rs b/codex-rs/config/src/diagnostics.rs index 3fc593dcdf..ca7df712d6 100644 --- a/codex-rs/config/src/diagnostics.rs +++ b/codex-rs/config/src/diagnostics.rs @@ -4,6 +4,7 @@ use crate::ConfigLayerEntry; use crate::ConfigLayerStack; use crate::ConfigLayerStackOrdering; +use crate::format_config_layer_source; use codex_app_server_protocol::ConfigLayerSource; use codex_utils_absolute_path::AbsolutePathBufGuard; use serde::de::DeserializeOwned; @@ -89,7 +90,6 @@ impl std::error::Error for ConfigLoadError { #[derive(Clone, Copy)] pub(crate) enum ConfigDiagnosticSource<'a> { Path(&'a Path), - #[cfg(any(target_os = "macos", test))] DisplayName(&'a str), } @@ -97,7 +97,6 @@ impl ConfigDiagnosticSource<'_> { pub(crate) fn to_path_buf(self) -> PathBuf { match self { ConfigDiagnosticSource::Path(path) => path.to_path_buf(), - #[cfg(any(target_os = "macos", test))] ConfigDiagnosticSource::DisplayName(name) => PathBuf::from(name), } } @@ -201,6 +200,27 @@ where I: IntoIterator, { for layer in layers { + if let Some(contents) = layer.raw_toml() { + let source_name = format_config_layer_source(&layer.name, config_toml_file); + let Some(base_dir) = layer.raw_toml_base_dir() else { + tracing::debug!( + "Skipping raw TOML diagnostics for {source_name} because it has no base directory" + ); + continue; + }; + // Match the base directory used when the raw non-file layer was + // parsed into the runtime layer so diagnostics resolve relative + // path fields with the same semantics. + let _absolute_path_base = AbsolutePathBufGuard::new(base_dir.as_path()); + if let Some(error) = config_error_from_typed_toml_for_source::( + ConfigDiagnosticSource::DisplayName(&source_name), + contents, + ) { + return Some(error); + } + continue; + } + let Some(path) = config_path_for_layer(layer, config_toml_file) else { continue; }; @@ -235,6 +255,7 @@ fn config_path_for_layer(layer: &ConfigLayerEntry, config_toml_file: &str) -> Op } ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => Some(file.to_path_buf()), ConfigLayerSource::Mdm { .. } + | ConfigLayerSource::EnterpriseManaged { .. } | ConfigLayerSource::SessionFlags | ConfigLayerSource::LegacyManagedConfigTomlFromMdm => None, } diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 0366962355..cb3984729b 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -1,4 +1,6 @@ +mod cloud_config_layers; mod cloud_requirements; +mod config_layer_source; mod config_requirements; pub mod config_toml; mod constraint; @@ -18,6 +20,7 @@ mod plugin_edit; pub mod profile_toml; mod project_root_markers; mod requirements_exec_policy; +mod requirements_layers; pub mod schema; mod skills_config; mod state; @@ -28,6 +31,10 @@ pub mod types; pub const CONFIG_TOML_FILE: &str = "config.toml"; +pub use cloud_config_layers::CloudConfigFragment; +pub use cloud_config_layers::CloudConfigFragmentSource; +pub use cloud_config_layers::CloudConfigLayerError; +pub use cloud_config_layers::cloud_config_layers_from_fragments; pub use cloud_requirements::CloudRequirementsLoadError; pub use cloud_requirements::CloudRequirementsLoadErrorCode; pub use cloud_requirements::CloudRequirementsLoader; @@ -35,6 +42,7 @@ pub use codex_app_server_protocol::ConfigLayerSource; pub use codex_protocol::config_types::ProfileV2Name; pub use codex_protocol::config_types::ProfileV2NameParseError; pub use codex_utils_absolute_path::AbsolutePathBuf; +pub use config_layer_source::format_config_layer_source; pub use config_requirements::AppRequirementToml; pub use config_requirements::AppToolRequirementToml; pub use config_requirements::AppToolsRequirementsToml; @@ -117,6 +125,8 @@ pub use requirements_exec_policy::RequirementsExecPolicyParseError; pub use requirements_exec_policy::RequirementsExecPolicyPatternTokenToml; pub use requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml; pub use requirements_exec_policy::RequirementsExecPolicyToml; +pub use requirements_layers::RequirementsLayerEntry; +pub use requirements_layers::compose_requirements; pub use skills_config::BundledSkillsConfig; pub use skills_config::SkillConfig; pub use skills_config::SkillsConfig; diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 3c1723e51b..a657af0f36 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -366,13 +366,18 @@ pub async fn load_config_layers_state( // paths, starting with `./`, but a path starting with `~/` _is_ a // supported use case. Because resolve_relative_paths_in_config_toml() // relies on AbsolutePathBufGuard to resolve `~/`, we must supply a - // value for base_dir, so codex_home is as good a value as any. - let managed_config = - resolve_relative_paths_in_config_toml(config.managed_config, codex_home)?; + // value for base_dir. Preserve that same base on the layer so later + // raw-TOML diagnostics parse with the same path semantics. + let raw_toml_base_dir = AbsolutePathBuf::from_absolute_path(codex_home)?; + let managed_config = resolve_relative_paths_in_config_toml( + config.managed_config, + raw_toml_base_dir.as_path(), + )?; layers.push(ConfigLayerEntry::new_with_raw_toml( ConfigLayerSource::LegacyManagedConfigTomlFromMdm, managed_config, config.raw_toml, + raw_toml_base_dir, )); } diff --git a/codex-rs/config/src/requirements_layers/hooks.rs b/codex-rs/config/src/requirements_layers/hooks.rs new file mode 100644 index 0000000000..94aefd3285 --- /dev/null +++ b/codex-rs/config/src/requirements_layers/hooks.rs @@ -0,0 +1,237 @@ +//! Hook events are append-only across requirements layers. The managed hook +//! directory is different: only one directory is usable on a given platform, so +//! conflicting values for the active platform fail closed. The inactive platform +//! field is first-filled to allow the same layer stack to carry OS-specific +//! directories. + +use crate::HookEventsToml; +use crate::ManagedHooksRequirementsToml; +use crate::RequirementSource; +use crate::Sourced; +use std::collections::BTreeMap; +use std::path::PathBuf; + +use super::stack::composition_conflict; +use super::stack::merge_output_source; + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +pub(super) enum HookDirectoryField { + #[default] + ManagedDir, + WindowsManagedDir, +} + +impl HookDirectoryField { + pub(super) fn current_platform() -> Self { + if cfg!(windows) { + Self::WindowsManagedDir + } else { + Self::ManagedDir + } + } + + fn field_name(self) -> &'static str { + match self { + Self::ManagedDir => "hooks.managed_dir", + Self::WindowsManagedDir => "hooks.windows_managed_dir", + } + } + + fn inactive(self) -> Self { + match self { + Self::ManagedDir => Self::WindowsManagedDir, + Self::WindowsManagedDir => Self::ManagedDir, + } + } +} + +pub(super) struct HookMergeState { + directory_field: HookDirectoryField, + dir_sources: BTreeMap, +} + +impl HookMergeState { + pub(super) fn new(directory_field: HookDirectoryField) -> Self { + Self { + directory_field, + dir_sources: BTreeMap::new(), + } + } + + pub(super) fn merge( + &mut self, + target: &mut Option>, + incoming: Option, + source: &RequirementSource, + ) -> Result<(), super::stack::RequirementsCompositionError> { + let Some(mut incoming) = incoming.filter(|value| !value.is_empty()) else { + return Ok(()); + }; + let Some(existing) = target.as_mut() else { + self.track_singleton_source( + HookDirectoryField::ManagedDir, + &incoming.managed_dir, + source, + ); + self.track_singleton_source( + HookDirectoryField::WindowsManagedDir, + &incoming.windows_managed_dir, + source, + ); + *target = Some(Sourced::new(incoming, source.clone())); + return Ok(()); + }; + + let active_field = self.directory_field; + let inactive_field = active_field.inactive(); + let incoming_active_dir = take_hook_dir(&mut incoming, active_field); + let incoming_inactive_dir = take_hook_dir(&mut incoming, inactive_field); + let mut changed = false; + changed |= self.merge_active_singleton( + active_field, + hook_dir_mut(&mut existing.value, active_field), + incoming_active_dir, + source, + )?; + changed |= self.fill_singleton( + inactive_field, + hook_dir_mut(&mut existing.value, inactive_field), + incoming_inactive_dir, + source, + ); + changed |= append_hook_events(&mut existing.value.hooks, incoming.hooks); + if changed { + merge_output_source(&mut existing.source, source); + } + Ok(()) + } + + fn track_singleton_source( + &mut self, + field: HookDirectoryField, + value: &Option, + source: &RequirementSource, + ) { + if value.is_some() { + self.dir_sources + .entry(field) + .or_insert_with(|| source.clone()); + } + } + + fn merge_active_singleton( + &mut self, + field: HookDirectoryField, + existing: &mut Option, + incoming: Option, + incoming_source: &RequirementSource, + ) -> Result { + let Some(incoming) = incoming else { + return Ok(false); + }; + + match existing { + Some(existing_value) if existing_value != &incoming => { + let existing_source = self + .dir_sources + .get(&field) + .cloned() + .unwrap_or_else(|| incoming_source.clone()); + Err(composition_conflict( + field.field_name().to_string(), + existing_source, + incoming_source.clone(), + format!( + "`{}` conflicts with `{}`", + existing_value.display(), + incoming.display() + ), + )) + } + Some(_) => Ok(false), + None => { + *existing = Some(incoming); + self.dir_sources + .entry(field) + .or_insert_with(|| incoming_source.clone()); + Ok(true) + } + } + } + + fn fill_singleton( + &mut self, + field: HookDirectoryField, + existing: &mut Option, + incoming: Option, + incoming_source: &RequirementSource, + ) -> bool { + if existing.is_none() + && let Some(incoming) = incoming + { + *existing = Some(incoming); + self.dir_sources + .entry(field) + .or_insert_with(|| incoming_source.clone()); + true + } else { + false + } + } +} + +fn take_hook_dir( + hooks: &mut ManagedHooksRequirementsToml, + field: HookDirectoryField, +) -> Option { + match field { + HookDirectoryField::ManagedDir => hooks.managed_dir.take(), + HookDirectoryField::WindowsManagedDir => hooks.windows_managed_dir.take(), + } +} + +fn hook_dir_mut( + hooks: &mut ManagedHooksRequirementsToml, + field: HookDirectoryField, +) -> &mut Option { + match field { + HookDirectoryField::ManagedDir => &mut hooks.managed_dir, + HookDirectoryField::WindowsManagedDir => &mut hooks.windows_managed_dir, + } +} + +fn append_hook_events(existing: &mut HookEventsToml, incoming: HookEventsToml) -> bool { + // Destructure without `..` so new hook events cannot be introduced without + // deciding whether requirements layer merging should append them. + let HookEventsToml { + pre_tool_use, + permission_request, + post_tool_use, + pre_compact, + post_compact, + session_start, + user_prompt_submit, + subagent_start, + subagent_stop, + stop, + } = incoming; + + let mut changed = false; + changed |= append_vec(&mut existing.pre_tool_use, pre_tool_use); + changed |= append_vec(&mut existing.permission_request, permission_request); + changed |= append_vec(&mut existing.post_tool_use, post_tool_use); + changed |= append_vec(&mut existing.pre_compact, pre_compact); + changed |= append_vec(&mut existing.post_compact, post_compact); + changed |= append_vec(&mut existing.session_start, session_start); + changed |= append_vec(&mut existing.user_prompt_submit, user_prompt_submit); + changed |= append_vec(&mut existing.subagent_start, subagent_start); + changed |= append_vec(&mut existing.subagent_stop, subagent_stop); + changed |= append_vec(&mut existing.stop, stop); + changed +} + +fn append_vec(existing: &mut Vec, mut incoming: Vec) -> bool { + let changed = !incoming.is_empty(); + existing.append(&mut incoming); + changed +} diff --git a/codex-rs/config/src/requirements_layers/layer.rs b/codex-rs/config/src/requirements_layers/layer.rs new file mode 100644 index 0000000000..d92024e117 --- /dev/null +++ b/codex-rs/config/src/requirements_layers/layer.rs @@ -0,0 +1,191 @@ +use crate::ConfigRequirementsToml; +use crate::ManagedHooksRequirementsToml; +use crate::RequirementSource; +use crate::RequirementsExecPolicyToml; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::AbsolutePathBufGuard; +use toml::Value as TomlValue; + +use super::stack::RequirementsCompositionError; + +#[derive(Clone, Debug)] +pub struct RequirementsLayerEntry { + pub(super) source: RequirementSource, + toml: RequirementsLayerToml, + base_dir: Option, +} + +impl RequirementsLayerEntry { + pub fn from_toml(source: RequirementSource, contents: impl Into) -> Self { + Self { + source, + toml: RequirementsLayerToml::String(contents.into()), + base_dir: None, + } + } + + pub fn from_toml_value(source: RequirementSource, value: TomlValue) -> Self { + Self { + source, + toml: RequirementsLayerToml::Value(value), + base_dir: None, + } + } + + pub fn with_base_dir(mut self, base_dir: AbsolutePathBuf) -> Self { + self.base_dir = Some(base_dir); + self + } +} + +#[derive(Clone, Debug)] +enum RequirementsLayerToml { + String(String), + Value(TomlValue), +} + +#[derive(Clone, Debug)] +pub(super) struct ComposableRequirementsLayer { + pub(super) source: RequirementSource, + pub(super) regular_toml: TomlValue, + pub(super) domain_fields: DomainMergedRequirementsFields, +} + +impl ComposableRequirementsLayer { + pub(super) fn from_entry( + layer: RequirementsLayerEntry, + hostname: Option<&str>, + ) -> Result { + let RequirementsLayerEntry { + source, + toml, + base_dir, + } = layer; + let (mut regular_toml, mut requirements) = { + let _guard = base_dir + .as_ref() + .map(|base_dir| AbsolutePathBufGuard::new(base_dir.as_path())); + let regular_toml = parse_layer_toml(&toml, &source)?; + let requirements = parse_layer_requirements(&toml, &source)?; + (regular_toml, requirements) + }; + + requirements.apply_remote_sandbox_config(hostname); + materialize_remote_sandbox_config(&mut regular_toml, &requirements)?; + strip_special_fields(&mut regular_toml); + + Ok(Self { + source, + regular_toml, + domain_fields: DomainMergedRequirementsFields { + rules: requirements.rules, + hooks: requirements.hooks, + permissions: requirements.permissions, + }, + }) + } +} + +#[derive(Clone, Debug)] +pub(super) struct DomainMergedRequirementsFields { + pub(super) rules: Option, + pub(super) hooks: Option, + pub(super) permissions: Option, +} + +fn parse_layer_toml( + toml: &RequirementsLayerToml, + source: &RequirementSource, +) -> Result { + match toml { + RequirementsLayerToml::String(contents) => { + toml::from_str(contents).map_err(|err: toml::de::Error| { + RequirementsCompositionError::Parse { + layer_source: source.clone(), + message: err.to_string(), + } + }) + } + RequirementsLayerToml::Value(value) => Ok(value.clone()), + } +} + +fn parse_layer_requirements( + toml: &RequirementsLayerToml, + source: &RequirementSource, +) -> Result { + match toml { + RequirementsLayerToml::String(contents) => { + toml::from_str(contents).map_err(|err: toml::de::Error| { + RequirementsCompositionError::Parse { + layer_source: source.clone(), + message: err.to_string(), + } + }) + } + RequirementsLayerToml::Value(value) => { + value.clone().try_into().map_err(|err: toml::de::Error| { + RequirementsCompositionError::Parse { + layer_source: source.clone(), + message: err.to_string(), + } + }) + } + } +} + +fn materialize_remote_sandbox_config( + layer_toml: &mut TomlValue, + requirements: &ConfigRequirementsToml, +) -> Result<(), RequirementsCompositionError> { + remove_top_level_field(layer_toml, "remote_sandbox_config"); + let Some(allowed_sandbox_modes) = requirements.allowed_sandbox_modes.as_ref() else { + return Ok(()); + }; + let Some(table) = layer_toml.as_table_mut() else { + return Ok(()); + }; + table.insert( + "allowed_sandbox_modes".to_string(), + toml_value_from_serializable(allowed_sandbox_modes)?, + ); + Ok(()) +} + +fn toml_value_from_serializable( + value: T, +) -> Result { + TomlValue::try_from(value).map_err(|err| RequirementsCompositionError::ComposedParse { + message: err.to_string(), + }) +} + +fn strip_special_fields(layer_toml: &mut TomlValue) { + remove_top_level_field(layer_toml, "rules"); + remove_top_level_field(layer_toml, "hooks"); + remove_nested_field_and_prune_empty(layer_toml, &["permissions", "filesystem", "deny_read"]); +} + +fn remove_top_level_field(value: &mut TomlValue, key: &str) -> Option { + value.as_table_mut()?.remove(key) +} + +fn remove_nested_field_and_prune_empty(value: &mut TomlValue, path: &[&str]) -> Option { + let (key, remaining) = path.split_first()?; + let table = value.as_table_mut()?; + if remaining.is_empty() { + return table.remove(*key); + } + + let removed = table + .get_mut(*key) + .and_then(|child| remove_nested_field_and_prune_empty(child, remaining)); + if table + .get(*key) + .and_then(TomlValue::as_table) + .is_some_and(toml::map::Map::is_empty) + { + table.remove(*key); + } + removed +} diff --git a/codex-rs/config/src/requirements_layers/mod.rs b/codex-rs/config/src/requirements_layers/mod.rs new file mode 100644 index 0000000000..52a23a29db --- /dev/null +++ b/codex-rs/config/src/requirements_layers/mod.rs @@ -0,0 +1,8 @@ +mod hooks; +mod layer; +mod permissions; +mod rules; +mod stack; + +pub use layer::RequirementsLayerEntry; +pub use stack::compose_requirements; diff --git a/codex-rs/config/src/requirements_layers/permissions.rs b/codex-rs/config/src/requirements_layers/permissions.rs new file mode 100644 index 0000000000..e4dd2f0ef2 --- /dev/null +++ b/codex-rs/config/src/requirements_layers/permissions.rs @@ -0,0 +1,82 @@ +//! `permissions.filesystem.deny_read` is intentionally additive across +//! requirements layers. Other `[permissions]` content stays in the regular TOML +//! merge path so permission profile tables follow config-style precedence. + +use crate::FilesystemDenyReadPattern; +use crate::RequirementSource; +use crate::Sourced; +use crate::config_requirements::FilesystemRequirementsToml; +use crate::config_requirements::PermissionsRequirementsToml; + +use super::stack::merge_output_source; + +#[derive(Default)] +pub(super) struct DenyReadMergeState { + deny_read: Vec, + source: Option, +} + +impl DenyReadMergeState { + pub(super) fn merge( + &mut self, + incoming: Option, + source: &RequirementSource, + ) { + let Some(incoming_deny_read) = incoming + .and_then(|permissions| permissions.filesystem) + .and_then(|filesystem| filesystem.deny_read) + .filter(|deny_read| !deny_read.is_empty()) + else { + return; + }; + + for pattern in incoming_deny_read { + if !self.deny_read.contains(&pattern) { + self.deny_read.push(pattern); + self.merge_source(source); + } + } + } + + pub(super) fn apply_to(self, target: &mut Option>) { + if self.deny_read.is_empty() { + return; + } + + let source = self.source.unwrap_or(RequirementSource::Unknown); + let Some(existing) = target.as_mut() else { + *target = Some(Sourced::new( + PermissionsRequirementsToml { + filesystem: Some(FilesystemRequirementsToml { + deny_read: Some(self.deny_read), + }), + profiles: Default::default(), + }, + source, + )); + return; + }; + + let filesystem = existing + .value + .filesystem + .get_or_insert_with(Default::default); + let deny_read = filesystem.deny_read.get_or_insert_with(Vec::new); + for pattern in self.deny_read { + if !deny_read.contains(&pattern) { + deny_read.push(pattern); + } + } + if existing.source != source { + existing.source = RequirementSource::composite([existing.source.clone(), source]); + } + } + + fn merge_source(&mut self, source: &RequirementSource) { + let Some(existing) = self.source.as_mut() else { + self.source = Some(source.clone()); + return; + }; + merge_output_source(existing, source); + } +} diff --git a/codex-rs/config/src/requirements_layers/rules.rs b/codex-rs/config/src/requirements_layers/rules.rs new file mode 100644 index 0000000000..1d73493a91 --- /dev/null +++ b/codex-rs/config/src/requirements_layers/rules.rs @@ -0,0 +1,26 @@ +//! Requirements rules are additive across layers. Higher-priority rules are +//! appended first so the final rule order keeps priority visible. + +use crate::RequirementSource; +use crate::RequirementsExecPolicyToml; +use crate::Sourced; + +use super::stack::merge_output_source; + +pub(super) fn merge( + target: &mut Option>, + incoming: Option, + source: &RequirementSource, +) { + let Some(incoming) = incoming else { + return; + }; + let Some(existing) = target.as_mut() else { + *target = Some(Sourced::new(incoming, source.clone())); + return; + }; + + let RequirementsExecPolicyToml { prefix_rules } = incoming; + existing.value.prefix_rules.extend(prefix_rules); + merge_output_source(&mut existing.source, source); +} diff --git a/codex-rs/config/src/requirements_layers/stack.rs b/codex-rs/config/src/requirements_layers/stack.rs new file mode 100644 index 0000000000..e93dd1c1da --- /dev/null +++ b/codex-rs/config/src/requirements_layers/stack.rs @@ -0,0 +1,286 @@ +//! Requirements layers are composed in the same order as config layers: lowest +//! precedence first, highest precedence last. Most fields use the same +//! TOML-level merge policy as config: lower-priority layers provide defaults, +//! and higher-priority layers override scalar/list values while recursively +//! extending tables. +//! +//! A few fields carry domain-specific meaning that raw TOML replacement would +//! break: +//! - `remote_sandbox_config` is evaluated within each layer before merging. +//! - `rules.prefix_rules` append high-priority rules first. +//! - `hooks` append high-priority event groups first while failing closed on +//! active managed-dir conflicts. +//! - `permissions.filesystem.deny_read` is a high-priority-first union across +//! layers. + +use crate::ConfigRequirementsToml; +use crate::ConfigRequirementsWithSources; +use crate::RequirementSource; +use crate::Sourced; +use crate::merge::merge_toml_values; +use std::io; +use thiserror::Error; +use toml::Value as TomlValue; + +use super::hooks::HookDirectoryField; +use super::hooks::HookMergeState; +use super::layer::ComposableRequirementsLayer; +use super::layer::RequirementsLayerEntry; +use super::permissions::DenyReadMergeState; + +#[derive(Debug, Error, PartialEq, Eq)] +pub enum RequirementsCompositionError { + #[error("failed to parse requirements layer {layer_source}: {message}")] + Parse { + layer_source: RequirementSource, + message: String, + }, + #[error("failed to parse merged requirements: {message}")] + ComposedParse { message: String }, + #[error( + "failed to compose requirements field `{field}` between {existing_source} and {incoming_source}: {message}" + )] + Conflict { + field: String, + existing_source: RequirementSource, + incoming_source: RequirementSource, + message: String, + }, +} + +impl From for io::Error { + fn from(error: RequirementsCompositionError) -> Self { + io::Error::new(io::ErrorKind::InvalidData, error) + } +} + +pub fn compose_requirements( + layers: impl IntoIterator, +) -> Result, RequirementsCompositionError> { + let hostname = crate::host_name(); + compose_requirements_for_hostname(layers, hostname.as_deref()) +} + +pub(super) fn compose_requirements_for_hostname( + layers: impl IntoIterator, + hostname: Option<&str>, +) -> Result, RequirementsCompositionError> { + compose_requirements_for_hostname_and_hook_directory( + layers, + hostname, + HookDirectoryField::current_platform(), + ) +} + +pub(super) fn compose_requirements_for_hostname_and_hook_directory( + layers: impl IntoIterator, + hostname: Option<&str>, + hook_directory_field: HookDirectoryField, +) -> Result, RequirementsCompositionError> { + let mut stack = RequirementsLayerStack::new(hook_directory_field); + for layer in layers { + stack.add_layer(layer, hostname)?; + } + stack.compose() +} + +struct RequirementsLayerStack { + layers: Vec, + hook_directory_field: HookDirectoryField, +} + +impl RequirementsLayerStack { + fn new(hook_directory_field: HookDirectoryField) -> Self { + Self { + layers: Vec::new(), + hook_directory_field, + } + } + + fn add_layer( + &mut self, + layer: RequirementsLayerEntry, + hostname: Option<&str>, + ) -> Result<(), RequirementsCompositionError> { + self.layers + .push(ComposableRequirementsLayer::from_entry(layer, hostname)?); + Ok(()) + } + + fn compose( + self, + ) -> Result, RequirementsCompositionError> { + let Self { + layers, + hook_directory_field, + } = self; + + let mut merged_toml = TomlValue::Table(toml::map::Map::new()); + for layer in &layers { + merge_toml_values(&mut merged_toml, &layer.regular_toml); + } + + let requirements: ConfigRequirementsToml = + merged_toml.try_into().map_err(|err: toml::de::Error| { + RequirementsCompositionError::ComposedParse { + message: err.to_string(), + } + })?; + let mut output = ConfigRequirementsWithSources::default(); + populate_merged_regular_fields_with_sources(&mut output, requirements, &layers); + let mut rules = None; + let mut hooks = HookMergeState::new(hook_directory_field); + let mut hooks_output = None; + let mut deny_read = DenyReadMergeState::default(); + // Regular TOML fields are folded low-to-high like config. These custom + // fields append or union values, so process them high-to-low to keep + // priority order visible in the output. + for layer in layers.iter().rev() { + let domain_fields = &layer.domain_fields; + super::rules::merge(&mut rules, domain_fields.rules.clone(), &layer.source); + hooks.merge( + &mut hooks_output, + domain_fields.hooks.clone(), + &layer.source, + )?; + deny_read.merge(domain_fields.permissions.clone(), &layer.source); + } + output.rules = rules; + output.hooks = hooks_output; + deny_read.apply_to(&mut output.permissions); + + let output_is_empty = output.clone().into_toml().is_empty(); + Ok((!output_is_empty).then_some(output)) + } +} + +fn populate_merged_regular_fields_with_sources( + output: &mut ConfigRequirementsWithSources, + requirements: ConfigRequirementsToml, + layers: &[ComposableRequirementsLayer], +) { + macro_rules! set_sourced { + ($field:ident, $keys:expr) => { + if let Some(value) = $field { + output.$field = Some(Sourced::new( + value, + source_for_top_level_keys(layers, $keys), + )); + } + }; + } + + // Destructure without `..` so every new requirements field must choose + // whether it belongs in the regular TOML merge path or in a special merger. + let ConfigRequirementsToml { + allowed_approval_policies, + allowed_approvals_reviewers, + allowed_sandbox_modes, + allowed_permissions, + remote_sandbox_config: _, + allowed_web_search_modes, + allow_managed_hooks_only, + allow_appshots, + computer_use, + windows, + feature_requirements, + hooks: _, + mcp_servers, + plugins, + apps, + rules: _, + enforce_residency, + network, + permissions, + guardian_policy_config, + } = requirements; + + set_sourced!(allowed_approval_policies, &["allowed_approval_policies"]); + set_sourced!( + allowed_approvals_reviewers, + &["allowed_approvals_reviewers"] + ); + set_sourced!(allowed_sandbox_modes, &["allowed_sandbox_modes"]); + set_sourced!(allowed_permissions, &["allowed_permissions"]); + set_sourced!(allowed_web_search_modes, &["allowed_web_search_modes"]); + set_sourced!(allow_managed_hooks_only, &["allow_managed_hooks_only"]); + set_sourced!(allow_appshots, &["allow_appshots"]); + set_sourced!(computer_use, &["computer_use"]); + set_sourced!(windows, &["windows"]); + set_sourced!(feature_requirements, &["features", "feature_requirements"]); + set_sourced!(mcp_servers, &["mcp_servers"]); + set_sourced!(plugins, &["plugins"]); + set_sourced!(apps, &["apps"]); + set_sourced!(enforce_residency, &["enforce_residency"]); + set_sourced!(network, &["experimental_network"]); + set_sourced!(permissions, &["permissions"]); + + if let Some(guardian_policy_config) = + guardian_policy_config.filter(|value| !value.trim().is_empty()) + { + output.guardian_policy_config = Some(Sourced::new( + guardian_policy_config, + source_for_top_level_keys(layers, &["guardian_policy_config"]), + )); + } +} + +fn source_for_top_level_keys( + layers: &[ComposableRequirementsLayer], + keys: &[&str], +) -> RequirementSource { + let matching_layers = layers + .iter() + .filter_map(|layer| { + top_level_value_for_keys(&layer.regular_toml, keys).map(|value| (&layer.source, value)) + }) + .collect::>(); + let Some((winning_source, winning_value)) = matching_layers.last() else { + return RequirementSource::Unknown; + }; + let winning_source = (*winning_source).clone(); + + if !winning_value.is_table() { + return winning_source; + } + + let table_sources = matching_layers + .into_iter() + .rev() + .filter_map(|(source, value)| value.is_table().then_some(source.clone())) + .collect::>(); + if table_sources.len() > 1 { + RequirementSource::composite(table_sources) + } else { + winning_source + } +} + +fn top_level_value_for_keys<'a>(value: &'a TomlValue, keys: &[&str]) -> Option<&'a TomlValue> { + let table = value.as_table()?; + keys.iter().find_map(|key| table.get(*key)) +} + +pub(super) fn merge_output_source(existing: &mut RequirementSource, incoming: &RequirementSource) { + if existing != incoming { + *existing = RequirementSource::composite([existing.clone(), incoming.clone()]); + } +} + +pub(super) fn composition_conflict( + field: String, + existing_source: RequirementSource, + incoming_source: RequirementSource, + message: impl Into, +) -> RequirementsCompositionError { + RequirementsCompositionError::Conflict { + field, + existing_source, + incoming_source, + message: message.into(), + } +} + +#[cfg(test)] +#[path = "stack_tests.rs"] +mod tests; diff --git a/codex-rs/config/src/requirements_layers/stack_tests.rs b/codex-rs/config/src/requirements_layers/stack_tests.rs new file mode 100644 index 0000000000..8acfd5f0d5 --- /dev/null +++ b/codex-rs/config/src/requirements_layers/stack_tests.rs @@ -0,0 +1,922 @@ +use super::super::RequirementsLayerEntry; +use super::super::hooks::HookDirectoryField; +use super::RequirementsCompositionError; +use super::compose_requirements_for_hostname; +use super::compose_requirements_for_hostname_and_hook_directory; +use crate::ConfigRequirementsToml; +use crate::ConfigRequirementsWithSources; +use crate::RequirementSource; +use crate::Sourced; +use codex_protocol::protocol::AskForApproval; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use std::collections::BTreeMap; + +fn layer(id: &str, name: &str, contents: &str) -> RequirementsLayerEntry { + RequirementsLayerEntry::from_toml( + RequirementSource::EnterpriseManaged { + id: id.to_string(), + name: name.to_string(), + }, + contents, + ) +} + +fn compose( + layers: Vec, +) -> Result, RequirementsCompositionError> { + Ok( + compose_requirements_for_hostname(layers, /*hostname*/ None)? + .map(ConfigRequirementsWithSources::into_toml), + ) +} + +fn compose_with_hook_directory_field( + layers: Vec, + hook_directory_field: HookDirectoryField, +) -> Result, RequirementsCompositionError> { + Ok(compose_requirements_for_hostname_and_hook_directory( + layers, + /*hostname*/ None, + hook_directory_field, + )? + .map(ConfigRequirementsWithSources::into_toml)) +} + +fn expected_requirements(contents: impl AsRef) -> ConfigRequirementsToml { + toml::from_str(contents.as_ref()).expect("parse expected requirements TOML") +} + +#[test] +fn empty_layers_compose_to_none() { + let composed = compose(Vec::new()).expect("compose empty layers"); + assert_eq!(composed, None); +} + +#[test] +fn top_level_values_use_toml_priority() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +allowed_approval_policies = ["on-request"] +allowed_sandbox_modes = ["workspace-write"] +"#, + ), + layer( + "req_high", + "High", + r#" +allowed_approval_policies = ["never"] +allowed_sandbox_modes = ["read-only"] +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +allowed_approval_policies = ["never"] +allowed_sandbox_modes = ["read-only"] +"# + ) + ); +} + +#[test] +fn composition_strategy_applies_to_non_cloud_layers() { + let mdm_source = RequirementSource::MdmManagedPreferences { + domain: "com.openai.codex".to_string(), + key: "requirements_toml_base64".to_string(), + }; + let system_file = if cfg!(windows) { + "C:\\requirements.toml" + } else { + "/etc/codex/requirements.toml" + }; + let system_source = RequirementSource::SystemRequirementsToml { + file: AbsolutePathBuf::from_absolute_path(system_file).expect("absolute path"), + }; + let high_path = if cfg!(windows) { + "C:\\secret" + } else { + "/secret" + }; + let low_path = if cfg!(windows) { + "C:\\other-secret" + } else { + "/other-secret" + }; + + let composed = compose_requirements_for_hostname( + vec![ + RequirementsLayerEntry::from_toml( + system_source, + format!( + r#" +allowed_approval_policies = ["on-request"] + +[features] +shared = false +system = true + +[[rules.prefix_rules]] +pattern = [{{ token = "npm" }}] +decision = "prompt" + +[permissions.filesystem] +deny_read = [{low_path:?}] +"# + ), + ), + RequirementsLayerEntry::from_toml( + mdm_source.clone(), + format!( + r#" +allowed_approval_policies = ["never"] + +[features] +shared = true + +[[rules.prefix_rules]] +pattern = [{{ token = "git" }}] +decision = "forbidden" + +[permissions.filesystem] +deny_read = [{high_path:?}] +"# + ), + ), + ], + /*hostname*/ None, + ) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed.clone().into_toml(), + expected_requirements(format!( + r#" +allowed_approval_policies = ["never"] + +[features] +shared = true +system = true + +[[rules.prefix_rules]] +pattern = [{{ token = "git" }}] +decision = "forbidden" + +[[rules.prefix_rules]] +pattern = [{{ token = "npm" }}] +decision = "prompt" + +[permissions.filesystem] +deny_read = [{high_path:?}, {low_path:?}] +"# + )) + ); + assert_eq!( + composed.allowed_approval_policies, + Some(Sourced::new(vec![AskForApproval::Never], mdm_source)) + ); +} + +#[test] +fn single_regular_layer_keeps_enterprise_managed_source() { + let composed = compose_requirements_for_hostname( + vec![layer( + "req_1", + "Security baseline", + r#" +allow_managed_hooks_only = true +"#, + )], + /*hostname*/ None, + ) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed.allow_managed_hooks_only, + Some(Sourced::new( + /*value*/ true, + RequirementSource::EnterpriseManaged { + id: "req_1".to_string(), + name: "Security baseline".to_string(), + }, + )) + ); +} + +#[test] +fn regular_toml_merge_recurses_into_tables() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[features] +beta = false +shared = false + +[apps.connector_1] +enabled = false + +[apps.connector_1.tools.search] +approval_mode = "prompt" + +[apps.connector_1.tools.list] +approval_mode = "prompt" +"#, + ), + layer( + "req_high", + "High", + r#" +[features] +alpha = true +shared = true + +[apps.connector_1] +enabled = true + +[apps.connector_1.tools.search] +approval_mode = "approve" +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[features] +alpha = true +beta = false +shared = true + +[apps.connector_1] +enabled = true + +[apps.connector_1.tools.list] +approval_mode = "prompt" + +[apps.connector_1.tools.search] +approval_mode = "approve" +"# + ) + ); +} + +#[test] +fn merged_table_source_is_composite_in_priority_order() { + let high_source = RequirementSource::EnterpriseManaged { + id: "req_high".to_string(), + name: "High".to_string(), + }; + let low_source = RequirementSource::EnterpriseManaged { + id: "req_low".to_string(), + name: "Low".to_string(), + }; + let composed = compose_requirements_for_hostname( + vec![ + RequirementsLayerEntry::from_toml( + low_source.clone(), + r#" +[features] +beta = true +"#, + ), + RequirementsLayerEntry::from_toml( + high_source.clone(), + r#" +[features] +alpha = true +"#, + ), + ], + /*hostname*/ None, + ) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed.feature_requirements.expect("features"), + Sourced::new( + crate::FeatureRequirementsToml { + entries: BTreeMap::from([("alpha".to_string(), true), ("beta".to_string(), true),]), + }, + RequirementSource::composite([high_source, low_source]), + ) + ); +} + +#[test] +fn mcp_requirements_use_regular_toml_merge() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[mcp_servers.shared.identity] +command = "low-mcp" + +[mcp_servers.low.identity] +url = "https://low.example.com/mcp" +"#, + ), + layer( + "req_high", + "High", + r#" +[mcp_servers.shared.identity] +command = "high-mcp" +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[mcp_servers.low.identity] +url = "https://low.example.com/mcp" + +[mcp_servers.shared.identity] +command = "high-mcp" +"# + ) + ); +} + +#[test] +fn network_maps_use_regular_toml_merge() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[experimental_network.domains] +"example.com" = "deny" +"low.example.com" = "deny" +"internal.example.com" = "allow" + +[experimental_network.unix_sockets] +"/tmp/shared.sock" = "deny" +"/tmp/low.sock" = "allow" +"/tmp/admin.sock" = "allow" +"#, + ), + layer( + "req_high", + "High", + r#" +[experimental_network.domains] +"example.com" = "allow" +"high.example.com" = "allow" +"internal.example.com" = "deny" + +[experimental_network.unix_sockets] +"/tmp/shared.sock" = "allow" +"/tmp/high.sock" = "allow" +"/tmp/admin.sock" = "deny" +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[experimental_network.domains] +"example.com" = "allow" +"high.example.com" = "allow" +"internal.example.com" = "deny" +"low.example.com" = "deny" + +[experimental_network.unix_sockets] +"/tmp/admin.sock" = "deny" +"/tmp/high.sock" = "allow" +"/tmp/low.sock" = "allow" +"/tmp/shared.sock" = "allow" +"# + ) + ); +} + +#[test] +fn windows_requirements_use_regular_toml_merge() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[windows] +allowed_sandbox_implementations = ["unelevated"] +"#, + ), + layer( + "req_high", + "High", + r#" +[windows] +allowed_sandbox_implementations = ["elevated"] +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[windows] +allowed_sandbox_implementations = ["elevated"] +"# + ) + ); +} + +#[test] +fn remote_sandbox_config_is_applied_per_layer() { + let composed = compose_requirements_for_hostname( + vec![ + layer( + "req_low", + "Low", + r#" +allowed_sandbox_modes = ["read-only"] +"#, + ), + layer( + "req_high", + "High", + r#" +[[remote_sandbox_config]] +hostname_patterns = ["build-*.example.com"] +allowed_sandbox_modes = ["workspace-write"] +"#, + ), + ], + Some("BUILD-01.EXAMPLE.COM."), + ) + .expect("compose requirements") + .expect("requirements present") + .into_toml(); + + assert_eq!( + composed, + expected_requirements( + r#" +allowed_sandbox_modes = ["workspace-write"] +"# + ) + ); +} + +#[test] +fn unmatched_remote_sandbox_config_does_not_shadow_lower_layers() { + let composed = compose_requirements_for_hostname( + vec![ + layer( + "req_low", + "Low", + r#" +allowed_sandbox_modes = ["read-only"] +"#, + ), + layer( + "req_high", + "High", + r#" +[[remote_sandbox_config]] +hostname_patterns = ["mac-*.example.com"] +allowed_sandbox_modes = ["workspace-write"] +"#, + ), + ], + Some("linux-01.example.com"), + ) + .expect("compose requirements") + .expect("requirements present") + .into_toml(); + + assert_eq!( + composed, + expected_requirements( + r#" +allowed_sandbox_modes = ["read-only"] +"# + ) + ); +} + +#[test] +fn rules_are_appended_in_priority_order() { + let composed = compose(vec![ + layer( + "req_low", + "Low", + r#" +[[rules.prefix_rules]] +pattern = [{ token = "npm" }] +decision = "prompt" +"#, + ), + layer( + "req_high", + "High", + r#" +[[rules.prefix_rules]] +pattern = [{ token = "git" }] +decision = "forbidden" +"#, + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[[rules.prefix_rules]] +pattern = [{ token = "git" }] +decision = "forbidden" + +[[rules.prefix_rules]] +pattern = [{ token = "npm" }] +decision = "prompt" +"# + ) + ); +} + +#[test] +fn hooks_append_groups_and_reject_conflicting_managed_dirs() { + let composed = compose_with_hook_directory_field( + vec![ + layer( + "req_low", + "Low", + r#" +[hooks] +managed_dir = "/managed/hooks" + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "low" +"#, + ), + layer( + "req_high", + "High", + r#" +[hooks] +managed_dir = "/managed/hooks" + +[[hooks.PreToolUse]] +matcher = "Edit" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "high" +"#, + ), + ], + HookDirectoryField::ManagedDir, + ) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[hooks] +managed_dir = "/managed/hooks" + +[[hooks.PreToolUse]] +matcher = "Edit" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "high" + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "low" +"# + ) + ); + + let err = compose_with_hook_directory_field( + vec![ + layer( + "req_low", + "Low", + r#" +[hooks] +managed_dir = "/managed/low" +"#, + ), + layer( + "req_high", + "High", + r#" +[hooks] +managed_dir = "/managed/high" +"#, + ), + ], + HookDirectoryField::ManagedDir, + ) + .expect_err("conflicting managed dirs should fail closed"); + assert!(err.to_string().contains("hooks.managed_dir")); + assert!(err.to_string().contains("High (req_high)")); + assert!(err.to_string().contains("Low (req_low)")); +} + +#[test] +fn active_windows_managed_dir_conflicts_fail_closed() { + let err = compose_with_hook_directory_field( + vec![ + layer( + "req_low", + "Low", + r#" +[hooks] +windows_managed_dir = 'C:\managed\low' +"#, + ), + layer( + "req_high", + "High", + r#" +[hooks] +windows_managed_dir = 'C:\managed\high' +"#, + ), + ], + HookDirectoryField::WindowsManagedDir, + ) + .expect_err("conflicting windows managed dirs should fail closed"); + + assert!(err.to_string().contains("hooks.windows_managed_dir")); + assert!(err.to_string().contains("High (req_high)")); + assert!(err.to_string().contains("Low (req_low)")); +} + +#[test] +fn inactive_hook_dir_conflicts_do_not_fail_composition() { + let composed = compose_with_hook_directory_field( + vec![ + layer( + "req_low", + "Low", + r#" +[hooks] +managed_dir = "/managed/hooks" +windows_managed_dir = 'C:\managed\low' + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "low" +"#, + ), + layer( + "req_high", + "High", + r#" +[hooks] +managed_dir = "/managed/hooks" +windows_managed_dir = 'C:\managed\high' + +[[hooks.PreToolUse]] +matcher = "Edit" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "high" +"#, + ), + ], + HookDirectoryField::ManagedDir, + ) + .expect("inactive windows managed dir conflict should not fail") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[hooks] +managed_dir = "/managed/hooks" +windows_managed_dir = 'C:\managed\high' + +[[hooks.PreToolUse]] +matcher = "Edit" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "high" + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "low" +"# + ) + ); + + let composed = compose_with_hook_directory_field( + vec![ + layer( + "req_low", + "Low", + r#" +[hooks] +managed_dir = "/managed/low" +windows_managed_dir = 'C:\managed\hooks' + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "low" +"#, + ), + layer( + "req_high", + "High", + r#" +[hooks] +managed_dir = "/managed/high" +windows_managed_dir = 'C:\managed\hooks' + +[[hooks.PreToolUse]] +matcher = "Edit" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "high" +"#, + ), + ], + HookDirectoryField::WindowsManagedDir, + ) + .expect("inactive managed dir conflict should not fail") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements( + r#" +[hooks] +managed_dir = "/managed/high" +windows_managed_dir = 'C:\managed\hooks' + +[[hooks.PreToolUse]] +matcher = "Edit" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "high" + +[[hooks.PreToolUse]] +matcher = "Bash" + +[[hooks.PreToolUse.hooks]] +type = "command" +command = "low" +"# + ) + ); +} + +#[test] +fn permissions_deny_read_unions_while_profiles_use_regular_toml_merge() { + let high_path = if cfg!(windows) { + "C:\\secret" + } else { + "/secret" + }; + let low_path = if cfg!(windows) { + "C:\\other-secret" + } else { + "/other-secret" + }; + let composed = compose(vec![ + layer( + "req_low", + "Low", + &format!( + r#" +[permissions.filesystem] +deny_read = [{high_path:?}, {low_path:?}] + +[permissions.managed-standard] +description = "Low profile" +extends = ":workspace" +"# + ), + ), + layer( + "req_high", + "High", + &format!( + r#" +[permissions.filesystem] +deny_read = [{high_path:?}] + +[permissions.managed-standard] +description = "High profile" +"# + ), + ), + ]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements(format!( + r#" +[permissions.filesystem] +deny_read = [{high_path:?}, {low_path:?}] + +[permissions.managed-standard] +description = "High profile" +extends = ":workspace" +"# + )) + ); +} + +#[test] +fn deny_read_only_layers_do_not_leave_empty_permissions_tables() { + let path = if cfg!(windows) { + "C:\\secret" + } else { + "/secret" + }; + let composed = compose(vec![layer( + "req_high", + "High", + &format!( + r#" +[permissions.filesystem] +deny_read = [{path:?}] +"# + ), + )]) + .expect("compose requirements") + .expect("requirements present"); + + assert_eq!( + composed, + expected_requirements(format!( + r#" +[permissions.filesystem] +deny_read = [{path:?}] +"# + )) + ); +} + +#[test] +fn parse_error_names_layer() { + let err = compose(vec![layer( + "req_bad", + "Bad layer", + "allowed_approval_policies = [1]", + )]) + .expect_err("invalid layer should fail"); + + assert!(err.to_string().contains("Bad layer (req_bad)")); + assert!(err.to_string().contains("allowed_approval_policies")); +} diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index 3ec0bfb314..e4d03c3d3c 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -97,33 +97,47 @@ impl LoaderOverrides { pub struct ConfigLayerEntry { pub name: ConfigLayerSource, pub config: TomlValue, - pub raw_toml: Option, pub version: String, pub disabled_reason: Option, + raw_toml: Option, hooks_config_folder_override: Option, } +#[derive(Debug, Clone, PartialEq)] +struct RawTomlLayer { + contents: String, + base_dir: AbsolutePathBuf, +} + impl ConfigLayerEntry { pub fn new(name: ConfigLayerSource, config: TomlValue) -> Self { let version = version_for_toml(&config); Self { name, config, - raw_toml: None, version, disabled_reason: None, + raw_toml: None, hooks_config_folder_override: None, } } - pub fn new_with_raw_toml(name: ConfigLayerSource, config: TomlValue, raw_toml: String) -> Self { + pub fn new_with_raw_toml( + name: ConfigLayerSource, + config: TomlValue, + raw_toml: String, + raw_toml_base_dir: AbsolutePathBuf, + ) -> Self { let version = version_for_toml(&config); Self { name, config, - raw_toml: Some(raw_toml), version, disabled_reason: None, + raw_toml: Some(RawTomlLayer { + contents: raw_toml, + base_dir: raw_toml_base_dir, + }), hooks_config_folder_override: None, } } @@ -137,9 +151,9 @@ impl ConfigLayerEntry { Self { name, config, - raw_toml: None, version, disabled_reason: Some(disabled_reason.into()), + raw_toml: None, hooks_config_folder_override: None, } } @@ -149,7 +163,13 @@ impl ConfigLayerEntry { } pub fn raw_toml(&self) -> Option<&str> { - self.raw_toml.as_deref() + self.raw_toml + .as_ref() + .map(|raw_toml| raw_toml.contents.as_str()) + } + + pub fn raw_toml_base_dir(&self) -> Option<&AbsolutePathBuf> { + self.raw_toml.as_ref().map(|raw_toml| &raw_toml.base_dir) } pub(crate) fn with_hooks_config_folder_override( @@ -181,6 +201,7 @@ impl ConfigLayerEntry { match &self.name { ConfigLayerSource::Mdm { .. } => None, ConfigLayerSource::System { file } => file.parent(), + ConfigLayerSource::EnterpriseManaged { .. } => None, ConfigLayerSource::User { file, .. } => file.parent(), ConfigLayerSource::Project { dot_codex_folder } => Some(dot_codex_folder.clone()), ConfigLayerSource::SessionFlags => None, diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 8663737ee5..d8b05c604f 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -349,6 +349,7 @@ fn skill_roots_from_layer_stack_inner( }); } ConfigLayerSource::Mdm { .. } + | ConfigLayerSource::EnterpriseManaged { .. } | ConfigLayerSource::SessionFlags | ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } | ConfigLayerSource::LegacyManagedConfigTomlFromMdm => {} diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 77d2c97d6e..d3473400d8 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -614,6 +614,9 @@ "unified_exec": { "type": "boolean" }, + "unified_exec_zsh_fork": { + "type": "boolean" + }, "use_legacy_landlock": { "type": "boolean" }, @@ -4731,6 +4734,9 @@ "unified_exec": { "type": "boolean" }, + "unified_exec_zsh_fork": { + "type": "boolean" + }, "use_legacy_landlock": { "type": "boolean" }, diff --git a/codex-rs/core/src/agent/agent_resolver.rs b/codex-rs/core/src/agent/agent_resolver.rs index eb806da3ae..115739c2fd 100644 --- a/codex-rs/core/src/agent/agent_resolver.rs +++ b/codex-rs/core/src/agent/agent_resolver.rs @@ -32,5 +32,5 @@ fn register_session_root(session: &Arc, turn: &Arc) { session .services .agent_control - .register_session_root(session.conversation_id, &turn.session_source); + .register_session_root(session.conversation_id, turn.parent_thread_id); } diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 12fe4f6bea..1f58fb437e 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -54,6 +54,7 @@ pub(crate) enum SpawnAgentForkMode { pub(crate) struct SpawnAgentOptions { pub(crate) fork_parent_spawn_call_id: Option, pub(crate) fork_mode: Option, + pub(crate) parent_thread_id: Option, pub(crate) environments: Option>, } @@ -262,12 +263,12 @@ impl AgentControl { .await? } (Some(session_source), None) => { - let forked_from_thread_id = thread_spawn_parent_thread_id(&session_source); Box::pin(state.spawn_new_thread_with_source( config.clone(), self.clone(), session_source, - forked_from_thread_id, + options.parent_thread_id, + /*forked_from_thread_id*/ None, /*thread_source*/ Some(ThreadSource::Subagent), /*persist_extended_history*/ false, /*metrics_service_name*/ None, @@ -309,6 +310,7 @@ impl AgentControl { } }; let thread_config = new_thread.thread.codex.thread_config_snapshot().await; + let parent_thread_id = thread_config.parent_thread_id; emit_subagent_session_started( &new_thread .thread @@ -319,7 +321,7 @@ impl AgentControl { client_metadata, new_thread.thread.codex.session.session_id(), new_thread.thread_id, - /*parent_thread_id*/ None, + parent_thread_id, thread_config, subagent_source.clone(), ); @@ -473,6 +475,7 @@ impl AgentControl { } if preserve_reference_context_item && config.features.enabled(Feature::MultiAgentV2) + && config.multi_agent_v2.usage_hint_enabled && let Some(subagent_usage_hint_text) = config.multi_agent_v2.subagent_usage_hint_text.clone() && let Some(subagent_usage_hint_message) = @@ -490,6 +493,7 @@ impl AgentControl { self.clone(), session_source, /*thread_source*/ Some(ThreadSource::Subagent), + /*parent_thread_id*/ Some(parent_thread_id), /*forked_from_thread_id*/ Some(parent_thread_id), /*persist_extended_history*/ false, inherited_shell_snapshot, @@ -638,6 +642,7 @@ impl AgentControl { .history .ok_or_else(|| CodexErr::ThreadNotFound(thread_id))? .items; + let parent_thread_id = stored_thread.parent_thread_id; let resumed_thread = state .resume_thread_with_history_with_source(ResumeThreadWithHistoryOptions { @@ -649,6 +654,7 @@ impl AgentControl { }), agent_control: self.clone(), session_source, + parent_thread_id, inherited_shell_snapshot, inherited_exec_policy, }) @@ -841,9 +847,9 @@ impl AgentControl { pub(crate) fn register_session_root( &self, current_thread_id: ThreadId, - current_session_source: &SessionSource, + current_parent_thread_id: Option, ) { - if thread_spawn_parent_thread_id(current_session_source).is_none() { + if current_parent_thread_id.is_none() { self.state.register_root_thread(current_thread_id); } } @@ -1218,7 +1224,8 @@ impl AgentControl { child_thread_id: ThreadId, session_source: Option<&SessionSource>, ) { - let Some(parent_thread_id) = session_source.and_then(thread_spawn_parent_thread_id) else { + let Some(parent_thread_id) = session_source.and_then(SessionSource::parent_thread_id) + else { return; }; let Some(state_db_ctx) = thread.state_db() else { @@ -1263,10 +1270,6 @@ impl AgentControl { } } -fn thread_spawn_parent_thread_id(session_source: &SessionSource) -> Option { - session_source.parent_thread_id() -} - fn agent_matches_prefix(agent_path: Option<&AgentPath>, prefix: &AgentPath) -> bool { if prefix.is_root() { return true; diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 43a6e47bfd..3e0242c037 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -74,6 +74,15 @@ fn assistant_message(text: &str, phase: Option) -> ResponseItem { } } +#[test] +fn register_session_root_skips_threads_with_explicit_parent() { + let control = AgentControl::default(); + + control.register_session_root(ThreadId::new(), Some(ThreadId::new())); + + assert_eq!(control.state.agent_id_for_path(&AgentPath::root()), None); +} + fn spawn_agent_call(call_id: &str) -> ResponseItem { ResponseItem::FunctionCall { id: None, @@ -708,6 +717,53 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() { "full-history forked child should preserve the parent diff baseline" ); + let mut disabled_hint_child_config = harness.config.clone(); + let _ = disabled_hint_child_config + .features + .enable(Feature::MultiAgentV2); + disabled_hint_child_config.multi_agent_v2.usage_hint_enabled = false; + disabled_hint_child_config + .multi_agent_v2 + .subagent_usage_hint_text = Some("Disabled child subagent guidance.".to_string()); + let disabled_hint_child_thread_id = harness + .control + .spawn_agent_with_metadata( + disabled_hint_child_config, + text_input("child task without hints"), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id, + depth: 1, + agent_path: None, + agent_nickname: None, + agent_role: None, + })), + SpawnAgentOptions { + fork_parent_spawn_call_id: Some(parent_spawn_call_id.clone()), + fork_mode: Some(SpawnAgentForkMode::FullHistory), + ..Default::default() + }, + ) + .await + .expect("forked spawn should honor disabled usage hints") + .thread_id; + let disabled_hint_child_thread = harness + .manager + .get_thread(disabled_hint_child_thread_id) + .await + .expect("disabled-hint child thread should be registered"); + let disabled_hint_history = disabled_hint_child_thread + .codex + .session + .clone_history() + .await; + assert!( + !history_contains_text( + disabled_hint_history.raw_items(), + "Disabled child subagent guidance.", + ), + "full-history forked child should not add subagent guidance when usage hints are disabled" + ); + let expected = ( child_thread_id, Op::UserInput { @@ -734,6 +790,11 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() { .shutdown_live_agent(child_thread_id) .await .expect("child shutdown should submit"); + let _ = harness + .control + .shutdown_live_agent(disabled_hint_child_thread_id) + .await + .expect("disabled-hint child shutdown should submit"); let _ = parent_thread .submit(Op::Shutdown {}) .await diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 620645d8f6..124dfb18c0 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -173,6 +173,7 @@ struct ModelClientState { provider: SharedModelProvider, auth_env_telemetry: AuthEnvTelemetry, session_source: SessionSource, + parent_thread_id: Option, model_verbosity: Option, enable_request_compression: bool, include_timing_metrics: bool, @@ -321,6 +322,7 @@ impl ModelClient { installation_id: String, provider_info: ModelProviderInfo, session_source: SessionSource, + parent_thread_id: Option, model_verbosity: Option, enable_request_compression: bool, include_timing_metrics: bool, @@ -344,6 +346,7 @@ impl ModelClient { provider: model_provider, auth_env_telemetry, session_source, + parent_thread_id, model_verbosity, enable_request_compression, include_timing_metrics, @@ -637,7 +640,7 @@ impl ModelClient { fn build_responses_identity_headers(&self) -> ApiHeaderMap { let mut extra_headers = self.build_subagent_headers(); - if let Some(parent_thread_id) = parent_thread_id_header_value(&self.state.session_source) + if let Some(parent_thread_id) = parent_thread_id_header_value(self.state.parent_thread_id) && let Ok(val) = HeaderValue::from_str(&parent_thread_id) { extra_headers.insert(X_CODEX_PARENT_THREAD_ID_HEADER, val); @@ -664,7 +667,7 @@ impl ModelClient { if let Some(subagent) = subagent_header_value(&self.state.session_source) { client_metadata.insert(X_OPENAI_SUBAGENT_HEADER.to_string(), subagent); } - if let Some(parent_thread_id) = parent_thread_id_header_value(&self.state.session_source) { + if let Some(parent_thread_id) = parent_thread_id_header_value(self.state.parent_thread_id) { client_metadata.insert( X_CODEX_PARENT_THREAD_ID_HEADER.to_string(), parent_thread_id, @@ -1733,10 +1736,8 @@ fn subagent_header_value(session_source: &SessionSource) -> Option { } } -fn parent_thread_id_header_value(session_source: &SessionSource) -> Option { - session_source - .parent_thread_id() - .map(|parent_thread_id| parent_thread_id.to_string()) +fn parent_thread_id_header_value(parent_thread_id: Option) -> Option { + parent_thread_id.map(|parent_thread_id| parent_thread_id.to_string()) } const RESPONSE_STREAM_CHANNEL_CAPACITY: usize = 1600; diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index b9d9172c83..f8d904036a 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -61,6 +61,13 @@ use tracing_subscriber::registry::LookupSpan; use tracing_subscriber::util::SubscriberInitExt; fn test_model_client(session_source: SessionSource) -> ModelClient { + test_model_client_with_parent(session_source, /*parent_thread_id*/ None) +} + +fn test_model_client_with_parent( + session_source: SessionSource, + parent_thread_id: Option, +) -> ModelClient { let provider = create_oss_provider_with_base_url("https://example.com/v1", WireApi::Responses); let thread_id = ThreadId::new(); ModelClient::new( @@ -70,6 +77,7 @@ fn test_model_client(session_source: SessionSource) -> ModelClient { /*installation_id*/ "11111111-1111-4111-8111-111111111111".to_string(), provider, session_source, + parent_thread_id, /*model_verbosity*/ None, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, @@ -272,13 +280,16 @@ fn build_subagent_headers_sets_internal_memory_consolidation_label() { #[test] fn build_ws_client_metadata_includes_window_lineage_and_turn_metadata() { let parent_thread_id = ThreadId::new(); - let client = test_model_client(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { - parent_thread_id, - depth: 2, - agent_path: None, - agent_nickname: None, - agent_role: None, - })); + let client = test_model_client_with_parent( + SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id, + depth: 2, + agent_path: None, + agent_nickname: None, + agent_role: None, + }), + Some(parent_thread_id), + ); client.advance_window_generation(); @@ -520,6 +531,7 @@ fn model_client_with_counting_attestation( /*installation_id*/ "11111111-1111-4111-8111-111111111111".to_string(), provider, SessionSource::Exec, + /*parent_thread_id*/ None, /*model_verbosity*/ None, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 3c55f63761..44a8c0ac6b 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -74,6 +74,8 @@ pub(crate) async fn run_codex_thread_interactive( ) -> Result { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_ops, rx_ops) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); + let conversation_history = initial_history.unwrap_or(InitialHistory::New); + let forked_from_thread_id = conversation_history.forked_from_id(); let CodexSpawnOk { codex, .. } = Box::pin(Codex::spawn(CodexSpawnArgs { config, installation_id: parent_session.installation_id.clone(), @@ -84,9 +86,10 @@ pub(crate) async fn run_codex_thread_interactive( plugins_manager: Arc::clone(&parent_session.services.plugins_manager), mcp_manager: Arc::clone(&parent_session.services.mcp_manager), extensions: Arc::clone(&parent_session.services.extensions), - conversation_history: initial_history.unwrap_or(InitialHistory::New), + conversation_history, session_source: SessionSource::SubAgent(subagent_source.clone()), - forked_from_thread_id: Some(parent_session.conversation_id), + forked_from_thread_id, + parent_thread_id: Some(parent_session.conversation_id), thread_source: Some(ThreadSource::Subagent), agent_control: parent_session.services.agent_control.clone(), dynamic_tools: Vec::new(), diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 12e0dcf3f2..d90f687fc6 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -7,6 +7,7 @@ use crate::session::SessionSettingsUpdate; use crate::session::SteerInputError; use codex_features::Feature; use codex_otel::SessionTelemetry; +use codex_protocol::ThreadId; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::Personality; @@ -67,6 +68,7 @@ pub struct ThreadConfigSnapshot { pub personality: Option, pub collaboration_mode: CollaborationMode, pub session_source: SessionSource, + pub parent_thread_id: Option, pub thread_source: Option, } @@ -262,6 +264,14 @@ impl CodexThread { self.codex.session.inject_if_running(items).await } + /// Starts a regular turn with model-visible items only if the thread is idle. + pub async fn try_start_turn_if_idle( + &self, + items: Vec, + ) -> Result<(), Vec> { + self.codex.session.try_start_turn_if_idle(items).await + } + pub async fn set_app_server_client_info( &self, app_server_client_name: Option, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 0730b513cd..bb2afcdab6 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -9856,7 +9856,55 @@ enabled = true assert_eq!(config.multi_agent_v2.max_wait_timeout_ms, 3_600_000); assert_eq!(config.multi_agent_v2.default_wait_timeout_ms, 30_000); assert_eq!(config.agent_max_threads, Some(3)); - assert!(!config.multi_agent_v2.non_code_mode_only); + assert_eq!( + config.multi_agent_v2.root_agent_usage_hint_text.as_deref(), + Some(DEFAULT_MULTI_AGENT_V2_ROOT_AGENT_USAGE_HINT_TEXT) + ); + assert!( + !config + .multi_agent_v2 + .root_agent_usage_hint_text + .as_deref() + .unwrap_or_default() + .contains("maximum concurrency"), + ); + assert_eq!( + config.multi_agent_v2.subagent_usage_hint_text.as_deref(), + Some(DEFAULT_MULTI_AGENT_V2_SUBAGENT_USAGE_HINT_TEXT) + ); + assert!( + !config + .multi_agent_v2 + .subagent_usage_hint_text + .as_deref() + .unwrap_or_default() + .contains("maximum concurrency"), + ); + assert!(config.multi_agent_v2.non_code_mode_only); + + Ok(()) +} + +#[tokio::test] +async fn multi_agent_v2_empty_usage_hint_overrides_clear_default_hints() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"[features.multi_agent_v2] +enabled = true +root_agent_usage_hint_text = "" +subagent_usage_hint_text = "" +"#, + )?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + assert_eq!(config.multi_agent_v2.root_agent_usage_hint_text, None); + assert_eq!(config.multi_agent_v2.subagent_usage_hint_text, None); Ok(()) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 3b8bc77d63..37be8a867d 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -186,6 +186,44 @@ pub(crate) const DEFAULT_MULTI_AGENT_V2_MAX_CONCURRENT_THREADS_PER_SESSION: usiz pub(crate) const DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS: i64 = 10_000; pub(crate) const DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS: i64 = 3600 * 1000; pub(crate) const DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS: i64 = 30_000; +const DEFAULT_MULTI_AGENT_V2_ROOT_AGENT_USAGE_HINT_TEXT: &str = r#"You are `/root`, the primary agent in a team of agents collaborating to fulfill the user's goals. + +At the start of your turn, you are the active agent. +You can spawn sub-agents to handle subtasks, and those sub-agents can spawn their own sub-agents. +All agents in the team, including the agents that you can assign tasks to, are equally intelligent and capable, and have access to the same set of tools. + +You can use `spawn_agent` to create a new agent, `assign_task` to give an existing agent a new task and trigger a turn, and `send_message` to pass a message to a running agent without triggering a turn. +Child agents can also spawn their own sub-agents. +You can decide how much context you want to propagate to your sub-agents with the `fork_turns` parameter. + +You will receive messages in the analysis channel in the form: +``` +Message Type: MESSAGE | FINAL_ANSWER +Sender: +Payload: + +``` +They may be addressed as to=/root +"#; +const DEFAULT_MULTI_AGENT_V2_SUBAGENT_USAGE_HINT_TEXT: &str = r#"You are an agent in a team of agents collaborating to complete a task. + +You can spawn sub-agents to handle subtasks, and those sub-agents can spawn their own sub-agents. All agents in the team, including the agents that you can assign tasks to, are equally intelligent and capable, and have access to the same set of tools. + +You can use `spawn_agent` to create a new agent, `assign_task` to give an existing agent a new task and trigger a turn, and `send_message` to pass a message to a running agent. +Child agents can also spawn their own sub-agents. + +When you provide a response in the final channel, that content is immediately delivered back to your parent agent. + +You will receive messages in the analysis channel in the form: +``` +Message Type: NEW_TASK | MESSAGE | FINAL_ANSWER +Task name: # only for NEW_TASK -- this determines your identity +Sender: +Payload: + +``` +You may also see them addressed as to=/root/..., which indicates your identity is /root/... +"#; pub(crate) const HARD_MIN_MULTI_AGENT_V2_TIMEOUT_MS: i64 = 0; pub(crate) const HARD_MAX_MULTI_AGENT_V2_TIMEOUT_MS: i64 = DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS; @@ -943,11 +981,15 @@ impl Default for MultiAgentV2Config { default_wait_timeout_ms: DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS, usage_hint_enabled: true, usage_hint_text: None, - root_agent_usage_hint_text: None, - subagent_usage_hint_text: None, + root_agent_usage_hint_text: Some( + DEFAULT_MULTI_AGENT_V2_ROOT_AGENT_USAGE_HINT_TEXT.to_string(), + ), + subagent_usage_hint_text: Some( + DEFAULT_MULTI_AGENT_V2_SUBAGENT_USAGE_HINT_TEXT.to_string(), + ), tool_namespace: None, hide_spawn_agent_metadata: false, - non_code_mode_only: false, + non_code_mode_only: true, } } } @@ -2145,14 +2187,14 @@ fn resolve_multi_agent_v2_config(config_toml: &ConfigToml) -> MultiAgentV2Config .and_then(|config| config.usage_hint_text.as_ref()) .cloned() .or(default.usage_hint_text); - let root_agent_usage_hint_text = base - .and_then(|config| config.root_agent_usage_hint_text.as_ref()) - .cloned() - .or(default.root_agent_usage_hint_text); - let subagent_usage_hint_text = base - .and_then(|config| config.subagent_usage_hint_text.as_ref()) - .cloned() - .or(default.subagent_usage_hint_text); + let root_agent_usage_hint_text = resolve_optional_prompt_text( + base.map(|config| &config.root_agent_usage_hint_text), + default.root_agent_usage_hint_text, + ); + let subagent_usage_hint_text = resolve_optional_prompt_text( + base.map(|config| &config.subagent_usage_hint_text), + default.subagent_usage_hint_text, + ); let tool_namespace = base .and_then(|config| config.tool_namespace.as_ref()) .cloned() @@ -2193,6 +2235,17 @@ fn resolve_terminal_resize_reflow_config(config_toml: &ConfigToml) -> TerminalRe } } +fn resolve_optional_prompt_text( + configured: Option<&Option>, + default: Option, +) -> Option { + match configured { + Some(Some(value)) if value.is_empty() => None, + Some(Some(value)) => Some(value.clone()), + Some(None) | None => default, + } +} + fn multi_agent_v2_toml_config(features: Option<&FeaturesToml>) -> Option<&MultiAgentV2ConfigToml> { match features?.multi_agent_v2.as_ref()? { FeatureToml::Enabled(_) => None, diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index db4343448f..8f53e8f8f5 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -156,8 +156,8 @@ pub(crate) fn is_guardian_reviewer_source( ) -> bool { matches!( session_source, - codex_protocol::protocol::SessionSource::SubAgent(SubAgentSource::Other(name)) - if name == GUARDIAN_REVIEWER_NAME + codex_protocol::protocol::SessionSource::SubAgent(SubAgentSource::Other(label)) + if label == GUARDIAN_REVIEWER_NAME ) } diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 80a1997477..336433fe16 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -704,6 +704,7 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); HookSource::SessionFlags => "session_flags", HookSource::Plugin => "plugin", HookSource::CloudRequirements => "cloud_requirements", + HookSource::CloudManagedConfig => "cloud_managed_config", HookSource::LegacyManagedConfigFile => "legacy_managed_config_file", HookSource::LegacyManagedConfigMdm => "legacy_managed_config_mdm", HookSource::Unknown => "unknown", diff --git a/codex-rs/core/src/personality_migration_tests.rs b/codex-rs/core/src/personality_migration_tests.rs index ba9a037d7a..ad42a41baa 100644 --- a/codex-rs/core/src/personality_migration_tests.rs +++ b/codex-rs/core/src/personality_migration_tests.rs @@ -45,6 +45,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R meta: SessionMeta { id: thread_id, forked_from_id: None, + parent_thread_id: None, timestamp: TEST_TIMESTAMP.to_string(), cwd: std::path::PathBuf::from("."), originator: "test_originator".to_string(), diff --git a/codex-rs/core/src/realtime_context_tests.rs b/codex-rs/core/src/realtime_context_tests.rs index d60484d2e7..8e74599897 100644 --- a/codex-rs/core/src/realtime_context_tests.rs +++ b/codex-rs/core/src/realtime_context_tests.rs @@ -32,6 +32,7 @@ fn stored_thread(cwd: &str, title: &str, first_user_message: &str) -> StoredThre thread_id: ThreadId::new(), rollout_path: Some(PathBuf::from("/tmp/rollout.jsonl")), forked_from_id: None, + parent_thread_id: None, preview: first_user_message.to_string(), name: (!title.is_empty()).then(|| title.to_string()), model_provider: "test-provider".to_string(), diff --git a/codex-rs/core/src/session/inject.rs b/codex-rs/core/src/session/inject.rs index 4a7189bbd6..64d4aaade1 100644 --- a/codex-rs/core/src/session/inject.rs +++ b/codex-rs/core/src/session/inject.rs @@ -1,7 +1,11 @@ use super::input_queue::TurnInput; use super::session::Session; use super::turn_context::TurnContext; +use crate::state::ActiveTurn; +use crate::state::TurnState; +use crate::tasks::RegularTask; use codex_protocol::models::ResponseItem; +use std::sync::Arc; impl Session { /// Returns the input if there is no active turn to inject into. @@ -28,6 +32,75 @@ impl Session { } } + /// Starts a regular turn with the provided items only if the session is idle. + pub(crate) async fn try_start_turn_if_idle( + self: &Arc, + input: Vec, + ) -> Result<(), Vec> { + if input.is_empty() { + return Ok(()); + } + if self.input_queue.has_trigger_turn_mailbox_items().await { + return Err(input); + } + + let turn_state = { + let mut active_turn = self.active_turn.lock().await; + if active_turn.is_some() { + return Err(input); + } + let active_turn = active_turn.get_or_insert_with(ActiveTurn::default); + Arc::clone(&active_turn.turn_state) + }; + + if self.input_queue.has_trigger_turn_mailbox_items().await { + self.clear_reserved_idle_turn(&turn_state).await; + self.maybe_start_turn_for_pending_work().await; + return Err(input); + } + + let turn_context = self + .new_default_turn_with_sub_id(uuid::Uuid::new_v4().to_string()) + .await; + self.maybe_emit_unknown_model_warning_for_turn(turn_context.as_ref()) + .await; + if self.input_queue.has_trigger_turn_mailbox_items().await { + self.clear_reserved_idle_turn(&turn_state).await; + self.maybe_start_turn_for_pending_work().await; + return Err(input); + } + let still_reserved = { + let active_turn = self.active_turn.lock().await; + active_turn.as_ref().is_some_and(|active_turn| { + active_turn.task.is_none() && Arc::ptr_eq(&active_turn.turn_state, &turn_state) + }) + }; + if !still_reserved { + self.clear_reserved_idle_turn(&turn_state).await; + return Err(input); + } + + self.input_queue + .extend_pending_input_for_turn_state( + turn_state.as_ref(), + input.into_iter().map(TurnInput::ResponseItem).collect(), + ) + .await; + self.start_task(turn_context, Vec::new(), RegularTask::new()) + .await; + Ok(()) + } + + async fn clear_reserved_idle_turn(&self, turn_state: &Arc>) { + let mut active_turn_guard = self.active_turn.lock().await; + if let Some(active_turn) = active_turn_guard.as_ref() + && active_turn.task.is_none() + && Arc::ptr_eq(&active_turn.turn_state, turn_state) + { + *active_turn_guard = None; + } + } + /// Injects items into active work, or records them without starting a turn. pub(crate) async fn inject_no_new_turn( &self, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index a9980ddde8..312af7d0c1 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -361,7 +361,6 @@ use codex_protocol::protocol::WarningEvent; use codex_protocol::user_input::UserInput; use codex_tools::ToolEnvironmentMode; use codex_tools::UnifiedExecShellMode; -use codex_tools::shell_command_backend_for_features; use codex_utils_absolute_path::AbsolutePathBuf; #[cfg(test)] use codex_utils_stream_parser::ProposedPlanSegment; @@ -401,6 +400,7 @@ pub(crate) struct CodexSpawnArgs { pub(crate) conversation_history: InitialHistory, pub(crate) session_source: SessionSource, pub(crate) forked_from_thread_id: Option, + pub(crate) parent_thread_id: Option, pub(crate) thread_source: Option, pub(crate) agent_control: AgentControl, pub(crate) dynamic_tools: Vec, @@ -466,6 +466,7 @@ impl Codex { conversation_history, session_source, forked_from_thread_id, + parent_thread_id, thread_source, agent_control, dynamic_tools, @@ -594,6 +595,7 @@ impl Codex { app_server_client_version: None, session_source, forked_from_thread_id, + parent_thread_id, thread_source, dynamic_tools, persist_extended_history, diff --git a/codex-rs/core/src/session/multi_agents.rs b/codex-rs/core/src/session/multi_agents.rs index 2cab13cecd..eb3da0da4b 100644 --- a/codex-rs/core/src/session/multi_agents.rs +++ b/codex-rs/core/src/session/multi_agents.rs @@ -12,6 +12,10 @@ pub(super) fn usage_hint_text<'a>( } let multi_agent_v2 = &turn_context.config.multi_agent_v2; + if !multi_agent_v2.usage_hint_enabled { + return None; + } + match session_source { SessionSource::SubAgent(SubAgentSource::ThreadSpawn { .. }) => { multi_agent_v2.subagent_usage_hint_text.as_deref() diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index 22f00d7433..38d6f64030 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -31,9 +31,8 @@ pub(super) async fn spawn_review_thread( .models_manager .list_models(RefreshStrategy::OnlineIfUncached) .await; - let shell_command_backend = shell_command_backend_for_features(review_features.get()); let unified_exec_shell_mode = UnifiedExecShellMode::for_session( - shell_command_backend, + codex_tools::unified_exec_uses_zsh_fork_for_features(review_features.get()), crate::tools::tool_user_shell_type(sess.services.user_shell.as_ref()), sess.services.shell_zsh_path.as_ref(), sess.services.main_execve_wrapper_exe.as_ref(), @@ -90,6 +89,7 @@ pub(super) async fn spawn_review_thread( sess.session_id().to_string(), sess.thread_id().to_string(), forked_from_thread_id, + parent_turn_context.parent_thread_id, &session_source, parent_turn_context.thread_source, review_turn_id.clone(), @@ -113,6 +113,7 @@ pub(super) async fn spawn_review_thread( reasoning_effort, reasoning_summary, session_source, + parent_thread_id: parent_turn_context.parent_thread_id, thread_source: parent_turn_context.thread_source, environments: parent_turn_context.environments.clone(), available_models, diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index dc623df263..800a14f153 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -106,6 +106,8 @@ pub(crate) struct SessionConfiguration { pub(super) session_source: SessionSource, /// Immediate history source copied into this thread, when this thread was forked. pub(super) forked_from_thread_id: Option, + /// Immediate control/spawn parent for this thread, when it has one. + pub(super) parent_thread_id: Option, /// Optional analytics source classification for this thread. pub(super) thread_source: Option, pub(super) dynamic_tools: Vec, @@ -182,6 +184,7 @@ impl SessionConfiguration { personality: self.personality, collaboration_mode: self.collaboration_mode.clone(), session_source: self.session_source.clone(), + parent_thread_id: self.parent_thread_id, thread_source: self.thread_source, } } @@ -499,6 +502,10 @@ impl Session { .forked_from_thread_id .or_else(|| initial_history.forked_from_id()); session_configuration.forked_from_thread_id = forked_from_id; + let parent_thread_id = session_configuration + .parent_thread_id + .or_else(|| initial_history.get_resumed_parent_thread_id()); + session_configuration.parent_thread_id = parent_thread_id; let event_persistence_mode = if session_configuration.persist_extended_history { ThreadEventPersistenceMode::Extended @@ -538,6 +545,7 @@ impl Session { CreateThreadParams { thread_id, forked_from_id, + parent_thread_id, source: session_source, thread_source: session_configuration.thread_source, base_instructions: BaseInstructions { @@ -1018,6 +1026,7 @@ impl Session { installation_id.clone(), session_configuration.provider.clone(), session_configuration.session_source.clone(), + session_configuration.parent_thread_id, config.model_verbosity, config.features.enabled(Feature::EnableRequestCompression), config.features.enabled(Feature::RuntimeMetrics), @@ -1027,7 +1036,7 @@ impl Session { .with_prompt_cache_key_override( crate::guardian::prompt_cache_key_override_for_review_session( &session_configuration.session_source, - session_configuration.forked_from_thread_id, + session_configuration.parent_thread_id, ), ), code_mode_service: crate::tools::code_mode::CodeModeService::new(), @@ -1070,6 +1079,7 @@ impl Session { session_id, thread_id, forked_from_id, + parent_thread_id, thread_source: session_configuration.thread_source, thread_name: session_configuration.thread_name.clone(), model: session_configuration.collaboration_mode.model().to_string(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 00a25fde8b..f3b4ad20c5 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -459,6 +459,7 @@ fn test_model_client_session() -> crate::client::ModelClientSession { /*installation_id*/ "11111111-1111-4111-8111-111111111111".to_string(), ModelProviderInfo::create_openai_provider(/* base_url */ /*base_url*/ None), codex_protocol::protocol::SessionSource::Exec, + /*parent_thread_id*/ None, /*model_verbosity*/ None, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, @@ -3116,6 +3117,7 @@ async fn set_rate_limits_retains_previous_credits() { app_server_client_version: None, session_source: SessionSource::Exec, forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -3221,6 +3223,7 @@ async fn set_rate_limits_updates_plan_type_when_present() { app_server_client_version: None, session_source: SessionSource::Exec, forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -3469,6 +3472,7 @@ async fn attach_thread_persistence(session: &mut Session) -> PathBuf { CreateThreadParams { thread_id: session.conversation_id, forked_from_id: None, + parent_thread_id: None, source: SessionSource::Exec, thread_source: None, base_instructions: BaseInstructions::default(), @@ -3749,6 +3753,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati app_server_client_version: None, session_source: SessionSource::Exec, forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -4493,6 +4498,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { app_server_client_version: None, session_source: SessionSource::Exec, forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -4603,6 +4609,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { app_server_client_version: None, session_source: SessionSource::Exec, forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -4697,6 +4704,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { /*installation_id*/ "11111111-1111-4111-8111-111111111111".to_string(), session_configuration.provider.clone(), session_configuration.session_source.clone(), + session_configuration.parent_thread_id, config.model_verbosity, config.features.enabled(Feature::EnableRequestCompression), config.features.enabled(Feature::RuntimeMetrics), @@ -4838,6 +4846,7 @@ async fn make_session_with_config_and_rx( app_server_client_version: None, session_source: SessionSource::Exec, forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -4942,6 +4951,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( app_server_client_version: None, session_source: session_source.clone(), forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -5964,6 +5974,7 @@ async fn shutdown_complete_does_not_append_to_thread_store_after_shutdown() { CreateThreadParams { thread_id: session.conversation_id, forked_from_id: None, + parent_thread_id: None, source: SessionSource::Exec, thread_source: None, base_instructions: BaseInstructions::default(), @@ -6446,6 +6457,7 @@ where app_server_client_version: None, session_source: SessionSource::Exec, forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, dynamic_tools, persist_extended_history: false, @@ -6540,6 +6552,7 @@ where /*installation_id*/ "11111111-1111-4111-8111-111111111111".to_string(), session_configuration.provider.clone(), session_configuration.session_source.clone(), + session_configuration.parent_thread_id, config.model_verbosity, config.features.enabled(Feature::EnableRequestCompression), config.features.enabled(Feature::RuntimeMetrics), @@ -7192,6 +7205,34 @@ async fn build_initial_context_omits_multi_agent_v2_usage_hints_when_feature_dis ); } +#[tokio::test] +async fn build_initial_context_omits_multi_agent_v2_usage_hints_when_hint_disabled() { + let (session, turn_context, _rx_event) = make_session_and_context_with_auth_and_config_and_rx( + CodexAuth::from_api_key("Test API Key"), + Vec::new(), + |config| { + let _ = config.features.enable(Feature::MultiAgentV2); + config.multi_agent_v2.usage_hint_enabled = false; + config.multi_agent_v2.root_agent_usage_hint_text = Some("Root guidance.".to_string()); + config.multi_agent_v2.subagent_usage_hint_text = Some("Subagent guidance.".to_string()); + }, + ) + .await; + + let initial_context = session.build_initial_context(turn_context.as_ref()).await; + + let developer_messages = developer_message_texts(&initial_context); + assert!( + !developer_messages.iter().any(|message| { + matches!( + message.as_slice(), + ["Root guidance."] | ["Subagent guidance."] + ) + }), + "did not expect multi-agent v2 usage hint developer messages, got {developer_messages:?}" + ); +} + #[tokio::test] async fn build_initial_context_omits_default_image_save_location_with_image_history() { let (session, turn_context) = make_session_and_context().await; @@ -8399,6 +8440,34 @@ async fn thread_idle_lifecycle_waits_for_trigger_turn_mailbox_work() { assert_eq!(0, calls.load(std::sync::atomic::Ordering::SeqCst)); } +#[tokio::test] +async fn try_start_turn_if_idle_rejects_active_turn_without_injecting() { + let (sess, tc, _rx) = make_session_and_context_with_rx().await; + sess.spawn_task( + Arc::clone(&tc), + Vec::new(), + NeverEndingTask { + kind: TaskKind::Regular, + listen_to_cancellation_token: true, + }, + ) + .await; + + let item = user_message("synthetic idle input"); + let err = sess + .try_start_turn_if_idle(vec![item.clone()]) + .await + .expect_err("active turn should reject idle-only input"); + + assert_eq!(vec![item], err); + assert_eq!( + Vec::::new(), + sess.input_queue.get_pending_input(&sess.active_turn).await + ); + + sess.abort_all_tasks(TurnAbortReason::Interrupted).await; +} + #[tokio::test] async fn steer_input_requires_active_turn() { let (sess, _tc, _rx) = make_session_and_context_with_rx().await; diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index dabf9c360e..a0625d65e2 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -693,6 +693,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { GUARDIAN_REVIEWER_NAME.to_string(), )), forked_from_thread_id: None, + parent_thread_id: None, thread_source: None, agent_control: AgentControl::default(), dynamic_tools: Vec::new(), diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index cf170007cc..5f506856e9 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -5,6 +5,7 @@ use crate::environment_selection::ResolvedTurnEnvironments; use codex_model_provider::SharedModelProvider; use codex_model_provider::create_model_provider; use codex_protocol::SessionId; +use codex_protocol::ThreadId; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::openai_models::ToolMode; use codex_protocol::protocol::ThreadSource; @@ -61,6 +62,7 @@ pub struct TurnContext { pub(crate) reasoning_effort: Option, pub(crate) reasoning_summary: ReasoningSummaryConfig, pub(crate) session_source: SessionSource, + pub(crate) parent_thread_id: Option, pub(crate) thread_source: Option, pub(crate) environments: ResolvedTurnEnvironments, /// The session's absolute working directory. All relative paths provided @@ -219,6 +221,7 @@ impl TurnContext { reasoning_effort, reasoning_summary: self.reasoning_summary, session_source: self.session_source.clone(), + parent_thread_id: self.parent_thread_id, thread_source: self.thread_source, environments: self.environments.clone(), #[allow(deprecated)] @@ -479,10 +482,8 @@ impl Session { let provider_for_context = create_model_provider(provider, auth_manager); let session_telemetry_for_context = session_telemetry; let available_models = models_manager.try_list_models().unwrap_or_default(); - let shell_command_backend = - shell_command_backend_for_features(per_turn_config.features.get()); let unified_exec_shell_mode = UnifiedExecShellMode::for_session( - shell_command_backend, + codex_tools::unified_exec_uses_zsh_fork_for_features(per_turn_config.features.get()), crate::tools::tool_user_shell_type(user_shell), shell_zsh_path, main_execve_wrapper_exe, @@ -508,6 +509,7 @@ impl Session { session_id.to_string(), thread_id.to_string(), session_configuration.forked_from_thread_id, + session_configuration.parent_thread_id, &session_configuration.session_source, session_configuration.thread_source, sub_id.clone(), @@ -531,6 +533,7 @@ impl Session { reasoning_effort, reasoning_summary, session_source, + parent_thread_id: session_configuration.parent_thread_id, thread_source: session_configuration.thread_source, environments, #[allow(deprecated)] diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 0a3a089414..98a3d01a71 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -187,6 +187,7 @@ pub(crate) struct ResumeThreadWithHistoryOptions { pub(crate) initial_history: InitialHistory, pub(crate) agent_control: AgentControl, pub(crate) session_source: SessionSource, + pub(crate) parent_thread_id: Option, pub(crate) inherited_shell_snapshot: Option>, pub(crate) inherited_exec_policy: Option>, } @@ -601,6 +602,7 @@ impl ThreadManager { Arc::clone(&self.state.auth_manager), self.agent_control(), session_source, + /*parent_thread_id*/ None, forked_from_thread_id, thread_source, options.dynamic_tools, @@ -685,6 +687,7 @@ impl ThreadManager { auth_manager, self.agent_control(), session_source, + /*parent_thread_id*/ None, /*forked_from_thread_id*/ None, thread_source, Vec::new(), @@ -713,6 +716,7 @@ impl ThreadManager { InitialHistory::New, Arc::clone(&self.state.auth_manager), self.agent_control(), + /*parent_thread_id*/ None, /*forked_from_thread_id*/ None, /*thread_source*/ None, Vec::new(), @@ -746,6 +750,7 @@ impl ThreadManager { auth_manager, self.agent_control(), session_source, + /*parent_thread_id*/ None, /*forked_from_thread_id*/ None, thread_source, Vec::new(), @@ -916,6 +921,7 @@ impl ThreadManager { history, Arc::clone(&self.state.auth_manager), self.agent_control(), + /*parent_thread_id*/ None, forked_from_thread_id, thread_source, Vec::new(), @@ -1039,6 +1045,7 @@ impl ThreadManagerState { config, agent_control, self.session_source.clone(), + /*parent_thread_id*/ None, /*forked_from_thread_id*/ None, /*thread_source*/ None, /*persist_extended_history*/ false, @@ -1056,6 +1063,7 @@ impl ThreadManagerState { config: Config, agent_control: AgentControl, session_source: SessionSource, + parent_thread_id: Option, forked_from_thread_id: Option, thread_source: Option, persist_extended_history: bool, @@ -1073,6 +1081,7 @@ impl ThreadManagerState { Arc::clone(&self.auth_manager), agent_control, session_source, + parent_thread_id, forked_from_thread_id, thread_source, Vec::new(), @@ -1096,6 +1105,7 @@ impl ThreadManagerState { initial_history, agent_control, session_source, + parent_thread_id, inherited_shell_snapshot, inherited_exec_policy, } = options; @@ -1108,6 +1118,7 @@ impl ThreadManagerState { Arc::clone(&self.auth_manager), agent_control, session_source, + parent_thread_id, /*forked_from_thread_id*/ None, thread_source, Vec::new(), @@ -1130,6 +1141,7 @@ impl ThreadManagerState { agent_control: AgentControl, session_source: SessionSource, thread_source: Option, + parent_thread_id: Option, forked_from_thread_id: Option, persist_extended_history: bool, inherited_shell_snapshot: Option>, @@ -1145,6 +1157,7 @@ impl ThreadManagerState { Arc::clone(&self.auth_manager), agent_control, session_source, + parent_thread_id, forked_from_thread_id, thread_source, Vec::new(), @@ -1167,6 +1180,7 @@ impl ThreadManagerState { initial_history: InitialHistory, auth_manager: Arc, agent_control: AgentControl, + parent_thread_id: Option, forked_from_thread_id: Option, thread_source: Option, dynamic_tools: Vec, @@ -1182,6 +1196,7 @@ impl ThreadManagerState { auth_manager, agent_control, self.session_source.clone(), + parent_thread_id, forked_from_thread_id, thread_source, dynamic_tools, @@ -1204,6 +1219,7 @@ impl ThreadManagerState { auth_manager: Arc, agent_control: AgentControl, session_source: SessionSource, + parent_thread_id: Option, forked_from_thread_id: Option, thread_source: Option, dynamic_tools: Vec, @@ -1258,6 +1274,7 @@ impl ThreadManagerState { conversation_history: initial_history, session_source, forked_from_thread_id, + parent_thread_id, thread_source, agent_control, dynamic_tools, diff --git a/codex-rs/core/src/tools/handlers/agent_jobs.rs b/codex-rs/core/src/tools/handlers/agent_jobs.rs index 4c3cd34c2e..821b3619c4 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs.rs @@ -211,6 +211,7 @@ async fn run_agent_job_loop( "agent_job:{job_id}" )))), SpawnAgentOptions { + parent_thread_id: Some(session.conversation_id), environments: Some(turn.environments.to_selections()), ..Default::default() }, diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index a6461b3581..c4120dd5af 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -124,6 +124,7 @@ async fn handle_spawn_agent( SpawnAgentOptions { fork_parent_spawn_call_id: args.fork_context.then(|| call_id.clone()), fork_mode: args.fork_context.then_some(SpawnAgentForkMode::FullHistory), + parent_thread_id: Some(session.conversation_id), environments: Some(turn.environments.to_selections()), }, )) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_spec.rs b/codex-rs/core/src/tools/handlers/multi_agents_spec.rs index ba74c22805..96acb595b4 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_spec.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_spec.rs @@ -11,9 +11,9 @@ use std::collections::BTreeMap; pub const MULTI_AGENT_V1_NAMESPACE: &str = "multi_agent_v1"; const MULTI_AGENT_V1_NAMESPACE_DESCRIPTION: &str = "Tools for spawning and managing sub-agents."; -const SPAWN_AGENT_INHERITED_MODEL_GUIDANCE: &str = "Spawned agents inherit your current model by default. If provided, `model` specifies the model to use for the spawned agent."; +const SPAWN_AGENT_INHERITED_MODEL_GUIDANCE: &str = "Spawned agents inherit your current model by default. Omit `model` to use that preferred default; set `model` only when an explicit override is needed."; const SPAWN_AGENT_MODEL_OVERRIDE_DESCRIPTION: &str = - "Model override for the new agent. Omit to inherit the parent model."; + "Model override for the new agent. Omit unless an explicit override is needed."; const SPAWN_AGENT_SERVICE_TIER_OVERRIDE_DESCRIPTION: &str = "Service tier override for the new agent. Omit unless explicitly requested."; const MAX_MODEL_OVERRIDES_IN_SPAWN_AGENT_DESCRIPTION: usize = 5; diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs index dfd5483a9c..6abf9a30cc 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/list_agents.rs @@ -30,7 +30,7 @@ impl ToolExecutor for Handler { session .services .agent_control - .register_session_root(session.conversation_id, &turn.session_source); + .register_session_root(session.conversation_id, turn.parent_thread_id); let agents = session .services .agent_control diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 9a4e39195d..f767f88d78 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -144,6 +144,7 @@ async fn handle_spawn_agent( SpawnAgentOptions { fork_parent_spawn_call_id: fork_mode.as_ref().map(|_| call_id.clone()), fork_mode, + parent_thread_id: Some(session.conversation_id), environments: Some(turn.environments.to_selections()), }, ), diff --git a/codex-rs/core/src/tools/handlers/shell_spec.rs b/codex-rs/core/src/tools/handlers/shell_spec.rs index 361f73bc04..13c531eaba 100644 --- a/codex-rs/core/src/tools/handlers/shell_spec.rs +++ b/codex-rs/core/src/tools/handlers/shell_spec.rs @@ -13,12 +13,15 @@ pub struct CommandToolOptions { #[cfg(test)] pub fn create_exec_command_tool(options: CommandToolOptions) -> ToolSpec { - create_exec_command_tool_with_environment_id(options, /*include_environment_id*/ false) + create_exec_command_tool_with_environment_id( + options, /*include_environment_id*/ false, /*include_shell_parameter*/ true, + ) } pub(crate) fn create_exec_command_tool_with_environment_id( options: CommandToolOptions, include_environment_id: bool, + include_shell_parameter: bool, ) -> ToolSpec { let mut properties = BTreeMap::from([ ( @@ -32,12 +35,6 @@ pub(crate) fn create_exec_command_tool_with_environment_id( .to_string(), )), ), - ( - "shell".to_string(), - JsonSchema::string(Some( - "Shell binary to launch. Defaults to the user's default shell.".to_string(), - )), - ), ( "tty".to_string(), JsonSchema::boolean(Some( @@ -58,6 +55,14 @@ pub(crate) fn create_exec_command_tool_with_environment_id( )), ), ]); + if include_shell_parameter { + properties.insert( + "shell".to_string(), + JsonSchema::string(Some( + "Shell binary to launch. Defaults to the user's default shell.".to_string(), + )), + ); + } if options.allow_login_shell { properties.insert( "login".to_string(), diff --git a/codex-rs/core/src/tools/handlers/shell_spec_tests.rs b/codex-rs/core/src/tools/handlers/shell_spec_tests.rs index c11f34e927..b7c0db7242 100644 --- a/codex-rs/core/src/tools/handlers/shell_spec_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_spec_tests.rs @@ -6,6 +6,13 @@ fn windows_shell_guidance_description() -> String { format!("\n\n{}", windows_shell_guidance()) } +fn has_parameter(tool: &ToolSpec, parameter_name: &str) -> bool { + serde_json::to_value(tool) + .expect("tool spec should serialize") + .pointer(&format!("/parameters/properties/{parameter_name}")) + .is_some() +} + #[test] fn exec_command_tool_matches_expected_spec() { let tool = create_exec_command_tool(CommandToolOptions { @@ -88,6 +95,21 @@ fn exec_command_tool_matches_expected_spec() { ); } +#[test] +fn exec_command_tool_can_hide_shell_parameter() { + let tool = create_exec_command_tool_with_environment_id( + CommandToolOptions { + allow_login_shell: true, + exec_permission_approvals_enabled: false, + }, + /*include_environment_id*/ false, + /*include_shell_parameter*/ false, + ); + + assert!(!has_parameter(&tool, "shell")); + assert!(has_parameter(&tool, "cmd")); +} + #[test] fn write_stdin_tool_matches_expected_spec() { let tool = create_write_stdin_tool(); diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 170c1bd72f..6fa2e40685 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -7,6 +7,7 @@ use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::hook_names::HookToolName; use crate::tools::registry::PostToolUsePayload; +use codex_exec_server::Environment; use codex_protocol::models::AdditionalPermissionProfile; use codex_tools::UnifiedExecShellMode; use serde::Deserialize; @@ -124,14 +125,33 @@ pub(crate) fn get_command( shell_type: shell.shell_type.clone(), }) } - UnifiedExecShellMode::ZshFork(zsh_fork_config) => Ok(ResolvedCommand { - command: vec![ - zsh_fork_config.shell_zsh_path.to_string_lossy().to_string(), - if use_login_shell { "-lc" } else { "-c" }.to_string(), - args.cmd.clone(), - ], - shell_type: ShellType::Zsh, - }), + UnifiedExecShellMode::ZshFork(zsh_fork_config) => { + if args.shell.is_some() { + return Err( + "`shell` is not supported for local zsh-fork exec; omit `shell` to use zsh-fork, or target a remote environment where `shell` is supported.".to_string(), + ); + } + + Ok(ResolvedCommand { + command: vec![ + zsh_fork_config.shell_zsh_path.to_string_lossy().to_string(), + if use_login_shell { "-lc" } else { "-c" }.to_string(), + args.cmd.clone(), + ], + shell_type: ShellType::Zsh, + }) + } + } +} + +pub(crate) fn shell_mode_for_environment( + turn_shell_mode: &UnifiedExecShellMode, + environment: &Environment, +) -> UnifiedExecShellMode { + if environment.is_remote() { + UnifiedExecShellMode::Direct + } else { + turn_shell_mode.clone() } } diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index 83236881f3..c26fa107c7 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -38,12 +38,14 @@ use super::ExecCommandArgs; use super::ExecCommandEnvironmentArgs; use super::get_command; use super::post_unified_exec_tool_use_payload; +use super::shell_mode_for_environment; #[derive(Clone, Copy)] pub(crate) struct ExecCommandHandlerOptions { pub(crate) allow_login_shell: bool, pub(crate) exec_permission_approvals_enabled: bool, pub(crate) include_environment_id: bool, + pub(crate) include_shell_parameter: bool, } pub struct ExecCommandHandler { @@ -57,6 +59,7 @@ impl Default for ExecCommandHandler { allow_login_shell: false, exec_permission_approvals_enabled: false, include_environment_id: false, + include_shell_parameter: true, }, } } @@ -81,6 +84,7 @@ impl ToolExecutor for ExecCommandHandler { exec_permission_approvals_enabled: self.options.exec_permission_approvals_enabled, }, self.options.include_environment_id, + self.options.include_shell_parameter, ) } @@ -140,10 +144,12 @@ impl ToolExecutor for ExecCommandHandler { ) .await; let process_id = manager.allocate_process_id().await; + let shell_mode = + shell_mode_for_environment(&turn.unified_exec_shell_mode, environment.as_ref()); let resolved_command = get_command( &args, session.user_shell(), - &turn.unified_exec_shell_mode, + &shell_mode, turn.config.permissions.allow_login_shell, ) .map_err(FunctionCallError::RespondToModel)?; @@ -260,6 +266,7 @@ impl ToolExecutor for ExecCommandHandler { cwd, sandbox_cwd: turn_environment.cwd.clone(), environment, + shell_mode, network: context.turn.network.clone(), tty, sandbox_permissions: effective_additional_permissions.sandbox_permissions, diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 0eb9ebbedb..b33a0e85b0 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::shell::ShellType; use crate::shell::default_user_shell; +use codex_exec_server::Environment; use codex_tools::UnifiedExecShellMode; use codex_tools::ZshForkConfig; use codex_utils_absolute_path::AbsolutePathBuf; @@ -165,7 +166,7 @@ fn test_get_command_rejects_explicit_login_when_disallowed() -> anyhow::Result<( } #[test] -fn test_get_command_ignores_explicit_shell_in_zsh_fork_mode() -> anyhow::Result<()> { +fn test_get_command_rejects_explicit_shell_in_zsh_fork_mode() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#; let args: ExecCommandArgs = parse_arguments(json)?; let shell_zsh_path = AbsolutePathBuf::from_absolute_path(if cfg!(windows) { @@ -174,7 +175,7 @@ fn test_get_command_ignores_explicit_shell_in_zsh_fork_mode() -> anyhow::Result< "/opt/codex/zsh" })?; let shell_mode = UnifiedExecShellMode::ZshFork(ZshForkConfig { - shell_zsh_path: shell_zsh_path.clone(), + shell_zsh_path, main_execve_wrapper_exe: AbsolutePathBuf::from_absolute_path(if cfg!(windows) { r"C:\opt\codex\codex-execve-wrapper" } else { @@ -182,23 +183,50 @@ fn test_get_command_ignores_explicit_shell_in_zsh_fork_mode() -> anyhow::Result< })?, }); - let resolved = get_command( + let err = get_command( &args, Arc::new(default_user_shell()), &shell_mode, /*allow_login_shell*/ true, ) - .map_err(anyhow::Error::msg)?; + .expect_err("explicit shell should be rejected"); + + assert!( + err.contains("`shell` is not supported for local zsh-fork exec"), + "unexpected error: {err}" + ); + Ok(()) +} + +#[tokio::test] +async fn shell_mode_for_environment_uses_direct_mode_for_remote_environments() -> anyhow::Result<()> +{ + let shell_zsh_path = AbsolutePathBuf::from_absolute_path(if cfg!(windows) { + r"C:\opt\codex\zsh" + } else { + "/opt/codex/zsh" + })?; + let shell_mode = UnifiedExecShellMode::ZshFork(ZshForkConfig { + shell_zsh_path, + main_execve_wrapper_exe: AbsolutePathBuf::from_absolute_path(if cfg!(windows) { + r"C:\opt\codex\codex-execve-wrapper" + } else { + "/opt/codex/codex-execve-wrapper" + })?, + }); + let local_environment = Environment::default_for_tests(); + let remote_environment = + Environment::create_for_tests(Some("ws://127.0.0.1:1/remote-exec-server".to_string()))?; assert_eq!( - resolved.command, - vec![ - shell_zsh_path.to_string_lossy().to_string(), - "-lc".to_string(), - "echo hello".to_string() - ] + shell_mode_for_environment(&shell_mode, &local_environment), + shell_mode ); - assert_eq!(resolved.shell_type, ShellType::Zsh); + assert_eq!( + shell_mode_for_environment(&shell_mode, &remote_environment), + UnifiedExecShellMode::Direct + ); + Ok(()) } diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 2017718f7d..b5d609ac1f 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -17,6 +17,7 @@ use crate::shell::ShellType; use crate::tools::runtimes::build_sandbox_command; use crate::tools::runtimes::exec_env_for_sandbox_permissions; use crate::tools::runtimes::prepend_zsh_fork_bin_to_path; +use crate::tools::sandboxing::ExecApprovalRequirement; use crate::tools::sandboxing::PermissionRequestPayload; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; @@ -100,6 +101,19 @@ fn approval_sandbox_permissions( } } +fn parent_approved_sandbox_override( + sandbox_permissions: SandboxPermissions, + additional_permissions_preapproved: bool, + exec_approval_requirement: &ExecApprovalRequirement, +) -> bool { + sandbox_permissions.requests_sandbox_override() + && (additional_permissions_preapproved + || matches!( + exec_approval_requirement, + ExecApprovalRequirement::NeedsApproval { .. } + )) +} + pub(super) async fn try_run_zsh_fork( req: &ShellRequest, attempt: &SandboxAttempt<'_>, @@ -228,6 +242,11 @@ pub(super) async fn try_run_zsh_fork( sandbox_permissions: req.sandbox_permissions, approval_sandbox_permissions, prompt_permissions: req.additional_permissions.clone(), + parent_sandbox_override_approved: parent_approved_sandbox_override( + req.sandbox_permissions, + req.additional_permissions_preapproved, + &req.exec_approval_requirement, + ), stopwatch: stopwatch.clone(), }; @@ -304,6 +323,11 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( req.additional_permissions_preapproved, ), prompt_permissions: req.additional_permissions.clone(), + parent_sandbox_override_approved: parent_approved_sandbox_override( + req.sandbox_permissions, + req.additional_permissions_preapproved, + &req.exec_approval_requirement, + ), stopwatch: Stopwatch::unlimited(), }; @@ -336,6 +360,7 @@ struct CoreShellActionProvider { sandbox_permissions: SandboxPermissions, approval_sandbox_permissions: SandboxPermissions, prompt_permissions: Option, + parent_sandbox_override_approved: bool, stopwatch: Stopwatch, } @@ -635,9 +660,22 @@ impl EscalationPolicy for CoreShellActionProvider { let decision_driven_by_policy = Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision); let unsandboxed_allowed = unsandboxed_execution_allowed(&self.file_system_sandbox_policy); - let needs_escalation = unsandboxed_allowed + let needs_unsandboxed_escalation = unsandboxed_allowed && (self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy); + // The parent shell/unified-exec approval already covered unmatched + // prompts caused by its sandbox override. Explicit exec-policy rules + // still keep their own decisions. + let decision = if self.parent_sandbox_override_approved + && evaluation.decision == Decision::Prompt + && !decision_driven_by_policy + { + Decision::Allow + } else { + evaluation.decision + }; + let needs_escalation = + self.sandbox_permissions.uses_additional_permissions() || needs_unsandboxed_escalation; let decision_source = if decision_driven_by_policy { DecisionSource::PrefixRule @@ -655,7 +693,7 @@ impl EscalationPolicy for CoreShellActionProvider { ), }; self.process_decision( - evaluation.decision, + decision, needs_escalation, program, argv, @@ -866,7 +904,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { EscalationExecution::Unsandboxed => PreparedExec { command, cwd: workdir.to_path_buf(), - env, + env: exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated), arg0: Some(first_arg.clone()), }, EscalationExecution::TurnDefault => { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 6e7a1ced54..07c27c9431 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -1,4 +1,5 @@ use super::CoreShellActionProvider; +use super::CoreShellCommandExecutor; use super::InterceptedExecPolicyContext; use super::ParsedShellCommand; use super::commands_for_intercepted_exec_policy; @@ -16,6 +17,9 @@ use codex_execpolicy::PolicyParser; use codex_execpolicy::RuleMatch; use codex_hooks::Hooks; use codex_hooks::HooksConfig; +use codex_network_proxy::PROXY_ACTIVE_ENV_KEY; +use codex_network_proxy::PROXY_ENV_KEYS; +use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; @@ -29,13 +33,16 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::GuardianCommandSource; use codex_sandboxing::SandboxType; +use codex_sandboxing::policy_transforms::effective_permission_profile; use codex_shell_escalation::EscalationExecution; use codex_shell_escalation::EscalationPermissions; use codex_shell_escalation::ExecResult; use codex_shell_escalation::ResolvedPermissionProfile; +use codex_shell_escalation::ShellCommandExecutor; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use serde_json::Value; +use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -146,6 +153,42 @@ fn approval_sandbox_permissions_only_downgrades_preapproved_additional_permissio ); } +#[test] +fn parent_approved_sandbox_override_tracks_parent_approval_sources() { + assert!(super::parent_approved_sandbox_override( + SandboxPermissions::RequireEscalated, + /*additional_permissions_preapproved*/ false, + &crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + )); + assert!(super::parent_approved_sandbox_override( + SandboxPermissions::WithAdditionalPermissions, + /*additional_permissions_preapproved*/ true, + &crate::tools::sandboxing::ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, + )); + assert!(!super::parent_approved_sandbox_override( + SandboxPermissions::UseDefault, + /*additional_permissions_preapproved*/ false, + &crate::tools::sandboxing::ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + )); + assert!(!super::parent_approved_sandbox_override( + SandboxPermissions::RequireEscalated, + /*additional_permissions_preapproved*/ false, + &crate::tools::sandboxing::ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + }, + )); +} + #[test] fn extract_shell_script_preserves_login_flag() { assert_eq!( @@ -348,6 +391,188 @@ fn shell_request_escalation_execution_is_explicit() { ); } +#[tokio::test] +async fn unsandboxed_intercepted_exec_strips_managed_network_env() -> anyhow::Result<()> { + let workdir = test_sandbox_cwd(); + let executor = CoreShellCommandExecutor { + command: Vec::new(), + cwd: workdir.clone(), + permission_profile: PermissionProfile::workspace_write(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox: SandboxType::None, + env: HashMap::new(), + network: None, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + arg0: None, + sandbox_policy_cwd: workdir.clone(), + windows_sandbox_workspace_roots: vec![workdir.clone()], + codex_linux_sandbox_exe: None, + use_legacy_landlock: false, + }; + let mut env = HashMap::new(); + env.insert(PROXY_ACTIVE_ENV_KEY.to_string(), "1".to_string()); + for key in PROXY_ENV_KEYS { + env.insert((*key).to_string(), format!("proxy-{key}")); + } + + let prepared = executor + .prepare_escalated_exec( + &AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?, + &["curl".to_string(), "example.com".to_string()], + &workdir, + env, + EscalationExecution::Unsandboxed, + ) + .await?; + + assert!(!prepared.env.contains_key(PROXY_ACTIVE_ENV_KEY)); + for key in PROXY_ENV_KEYS { + assert!(!prepared.env.contains_key(*key)); + } + + Ok(()) +} + +#[tokio::test] +async fn preapproved_additional_permissions_escalate_intercepted_exec() -> anyhow::Result<()> { + let (session, turn_context) = make_session_and_context().await; + let requested_permissions = AdditionalPermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + Some(vec![ + AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(), + ]), + )), + ..Default::default() + }; + let workdir = test_sandbox_cwd(); + let permission_profile = effective_permission_profile( + &PermissionProfile::workspace_write(), + Some(&requested_permissions), + ); + let provider = CoreShellActionProvider { + policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())), + session: Arc::new(session), + turn: Arc::new(turn_context), + call_id: "preapproved-additional-permissions".to_string(), + tool_name: GuardianCommandSource::Shell, + approval_policy: AskForApproval::OnRequest, + permission_profile: permission_profile.clone(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + sandbox_policy_cwd: workdir.clone(), + sandbox_permissions: SandboxPermissions::WithAdditionalPermissions, + approval_sandbox_permissions: SandboxPermissions::UseDefault, + prompt_permissions: Some(requested_permissions), + parent_sandbox_override_approved: true, + stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), + }; + + let action = codex_shell_escalation::EscalationPolicy::determine_action( + &provider, + &AbsolutePathBuf::from_absolute_path("/usr/bin/printf")?, + &["printf".to_string(), "hello".to_string()], + &workdir, + ) + .await?; + + let expected = codex_shell_escalation::EscalationDecision::Escalate( + EscalationExecution::Permissions(EscalationPermissions::ResolvedPermissionProfile( + ResolvedPermissionProfile { permission_profile }, + )), + ); + assert_eq!(action, expected); + + Ok(()) +} + +#[tokio::test] +async fn parent_require_escalated_approval_escalates_intercepted_exec() -> anyhow::Result<()> { + let (session, turn_context) = make_session_and_context().await; + let workdir = test_sandbox_cwd(); + let provider = CoreShellActionProvider { + policy: Arc::new(RwLock::new(codex_execpolicy::Policy::empty())), + session: Arc::new(session), + turn: Arc::new(turn_context), + call_id: "parent-require-escalated".to_string(), + tool_name: GuardianCommandSource::Shell, + approval_policy: AskForApproval::OnRequest, + permission_profile: PermissionProfile::workspace_write(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + sandbox_policy_cwd: workdir.clone(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + approval_sandbox_permissions: SandboxPermissions::RequireEscalated, + prompt_permissions: None, + parent_sandbox_override_approved: true, + stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), + }; + + let action = codex_shell_escalation::EscalationPolicy::determine_action( + &provider, + &AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?, + &["curl".to_string(), "example.com".to_string()], + &workdir, + ) + .await?; + + assert_eq!( + action, + codex_shell_escalation::EscalationDecision::Escalate(EscalationExecution::Unsandboxed) + ); + + Ok(()) +} + +#[tokio::test] +async fn parent_approval_does_not_override_intercepted_exec_policy_prompt() -> anyhow::Result<()> { + let (session, turn_context) = make_session_and_context().await; + let mut parser = PolicyParser::new(); + parser.parse( + "test.rules", + r#"prefix_rule(pattern = ["curl"], decision = "prompt")"#, + )?; + let workdir = test_sandbox_cwd(); + let provider = CoreShellActionProvider { + policy: Arc::new(RwLock::new(parser.build())), + session: Arc::new(session), + turn: Arc::new(turn_context), + call_id: "parent-policy-prompt".to_string(), + tool_name: GuardianCommandSource::Shell, + approval_policy: AskForApproval::Granular(GranularApprovalConfig { + sandbox_approval: true, + rules: false, + skill_approval: true, + request_permissions: true, + mcp_elicitations: true, + }), + permission_profile: PermissionProfile::workspace_write(), + file_system_sandbox_policy: read_only_file_system_sandbox_policy(), + sandbox_policy_cwd: workdir.clone(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + approval_sandbox_permissions: SandboxPermissions::RequireEscalated, + prompt_permissions: None, + parent_sandbox_override_approved: true, + stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), + }; + + let action = codex_shell_escalation::EscalationPolicy::determine_action( + &provider, + &AbsolutePathBuf::from_absolute_path("/usr/bin/curl")?, + &["curl".to_string(), "example.com".to_string()], + &workdir, + ) + .await?; + + assert_eq!( + action, + codex_shell_escalation::EscalationDecision::Deny { + reason: Some("Execution forbidden by policy".to_string()), + } + ); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Result<()> { let (session, mut turn_context) = make_session_and_context().await; @@ -460,6 +685,7 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul sandbox_permissions: SandboxPermissions::RequireEscalated, approval_sandbox_permissions: SandboxPermissions::RequireEscalated, prompt_permissions: None, + parent_sandbox_override_approved: false, stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), }; @@ -677,6 +903,7 @@ prefix_rule(pattern = ["{cat_path_literal}"], decision = "allow") sandbox_permissions: SandboxPermissions::UseDefault, approval_sandbox_permissions: SandboxPermissions::UseDefault, prompt_permissions: None, + parent_sandbox_override_approved: false, stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), }; @@ -720,6 +947,7 @@ async fn denied_reads_keep_granular_sandbox_rejection_for_escalation() -> anyhow sandbox_permissions: SandboxPermissions::RequireEscalated, approval_sandbox_permissions: SandboxPermissions::RequireEscalated, prompt_permissions: None, + parent_sandbox_override_approved: false, stopwatch: codex_shell_escalation::Stopwatch::new(Duration::from_secs(1)), }; diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index e49c925728..5fbb432683 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -46,6 +46,7 @@ use codex_protocol::error::CodexErr; use codex_protocol::error::SandboxErr; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::protocol::ReviewDecision; +use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; use codex_shell_command::powershell::prefix_powershell_script_with_utf8; use codex_tools::UnifiedExecShellMode; @@ -118,6 +119,65 @@ impl<'a> UnifiedExecRuntime<'a> { shell_mode, } } + + fn first_attempt_sandbox_permissions(&self, req: &UnifiedExecRequest) -> SandboxPermissions { + if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_)) + && req.sandbox_permissions.requires_escalated_permissions() + { + SandboxPermissions::UseDefault + } else { + req.sandbox_permissions + } + } + + fn first_attempt_exec_approval_requirement( + &self, + req: &UnifiedExecRequest, + ) -> ExecApprovalRequirement { + if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_)) + && let ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment, + } = &req.exec_approval_requirement + { + ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: proposed_execpolicy_amendment.clone(), + } + } else { + req.exec_approval_requirement.clone() + } + } + + fn launch_sandbox_permissions( + &self, + req: &UnifiedExecRequest, + attempt: &SandboxAttempt<'_>, + ) -> SandboxPermissions { + if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_)) + && attempt.sandbox != SandboxType::None + && req.sandbox_permissions.requires_escalated_permissions() + { + SandboxPermissions::UseDefault + } else { + req.sandbox_permissions + } + } + + fn launch_additional_permissions( + &self, + req: &UnifiedExecRequest, + attempt: &SandboxAttempt<'_>, + ) -> Option { + if matches!(&self.shell_mode, UnifiedExecShellMode::ZshFork(_)) + && attempt.sandbox != SandboxType::None + && req.sandbox_permissions.requires_escalated_permissions() + { + None + } else { + req.additional_permissions.clone() + } + } } impl Sandboxable for UnifiedExecRuntime<'_> { @@ -203,7 +263,7 @@ impl Approvable for UnifiedExecRuntime<'_> { &self, req: &UnifiedExecRequest, ) -> Option { - Some(req.exec_approval_requirement.clone()) + Some(self.first_attempt_exec_approval_requirement(req)) } fn permission_request_payload( @@ -217,7 +277,7 @@ impl Approvable for UnifiedExecRuntime<'_> { } fn sandbox_permissions(&self, req: &UnifiedExecRequest) -> SandboxPermissions { - req.sandbox_permissions + self.first_attempt_sandbox_permissions(req) } } @@ -264,17 +324,15 @@ impl<'a> ToolRuntime for UnifiedExecRunt let base_command = &req.command; let session_shell = ctx.session.user_shell(); let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions(); - let sandbox_permissions = sandbox_permissions_preserving_denied_reads( - req.sandbox_permissions, + let launch_sandbox_permissions = sandbox_permissions_preserving_denied_reads( + self.launch_sandbox_permissions(req, attempt), &file_system_sandbox_policy, ); - let req = &UnifiedExecRequest { - sandbox_permissions, - ..req.clone() - }; - let managed_network = - managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions); - let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions); + let managed_network = managed_network_for_sandbox_permissions( + req.network.as_ref(), + launch_sandbox_permissions, + ); + let mut env = exec_env_for_sandbox_permissions(&req.env, launch_sandbox_permissions); if let Some(network) = managed_network { network.apply_to_env(&mut env); } @@ -316,9 +374,13 @@ impl<'a> ToolRuntime for UnifiedExecRunt }; if let UnifiedExecShellMode::ZshFork(zsh_fork_config) = &self.shell_mode { - let command = - build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone()) - .map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?; + let command = build_sandbox_command( + &command, + &req.cwd, + &env, + self.launch_additional_permissions(req, attempt), + ) + .map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?; let options = unified_exec_options(attempt.network_denial_cancellation_token.clone()); let mut exec_env = attempt .env_for(command, options, managed_network) @@ -400,8 +462,15 @@ impl<'a> ToolRuntime for UnifiedExecRunt mod tests { use super::*; use crate::exec::DEFAULT_EXEC_COMMAND_TIMEOUT_MS; + use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolRuntime; use codex_exec_server::Environment; + use codex_protocol::config_types::WindowsSandboxLevel; + use codex_protocol::models::NetworkPermissions; + use codex_protocol::models::PermissionProfile; + use codex_sandboxing::SandboxManager; + use codex_sandboxing::SandboxType; + use codex_tools::ZshForkConfig; use std::time::Duration; use tempfile::tempdir; @@ -463,4 +532,149 @@ mod tests { assert_eq!(runtime.sandbox_cwd(&request), Some(&sandbox_cwd)); } + + #[tokio::test] + async fn zsh_fork_first_attempt_uses_turn_default_sandbox_permissions() { + let manager = UnifiedExecProcessManager::default(); + let request = test_request( + SandboxPermissions::RequireEscalated, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + ); + let direct_runtime = UnifiedExecRuntime::new(&manager, UnifiedExecShellMode::Direct); + let zsh_fork_runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode()); + + assert_eq!( + direct_runtime.sandbox_permissions(&request), + SandboxPermissions::RequireEscalated + ); + assert_eq!( + zsh_fork_runtime.sandbox_permissions(&request), + SandboxPermissions::UseDefault + ); + } + + #[tokio::test] + async fn zsh_fork_first_attempt_preserves_additional_permissions_request() { + let manager = UnifiedExecProcessManager::default(); + let request = test_request( + SandboxPermissions::WithAdditionalPermissions, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + ); + let zsh_fork_runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode()); + + assert_eq!( + zsh_fork_runtime.sandbox_permissions(&request), + SandboxPermissions::WithAdditionalPermissions + ); + } + + #[tokio::test] + async fn zsh_fork_sandboxed_launch_preserves_additional_permissions() { + let manager = UnifiedExecProcessManager::default(); + let runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode()); + let additional_permissions = AdditionalPermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + ..Default::default() + }; + let mut request = test_request( + SandboxPermissions::WithAdditionalPermissions, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + ); + request.additional_permissions = Some(additional_permissions.clone()); + let cwd = request.sandbox_cwd.clone(); + let workspace_roots = vec![cwd.clone()]; + let sandbox_manager = SandboxManager::new(); + let permissions = PermissionProfile::workspace_write(); + let attempt = SandboxAttempt { + sandbox: SandboxType::MacosSeatbelt, + permissions: &permissions, + enforce_managed_network: true, + manager: &sandbox_manager, + sandbox_cwd: &cwd, + workspace_roots: workspace_roots.as_slice(), + codex_linux_sandbox_exe: None, + use_legacy_landlock: false, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + windows_sandbox_private_desktop: false, + network_denial_cancellation_token: None, + }; + + assert_eq!( + runtime.launch_sandbox_permissions(&request, &attempt), + SandboxPermissions::WithAdditionalPermissions + ); + assert_eq!( + runtime.launch_additional_permissions(&request, &attempt), + Some(additional_permissions) + ); + } + + #[tokio::test] + async fn zsh_fork_execpolicy_allow_does_not_bypass_trampoline_sandbox() { + let manager = UnifiedExecProcessManager::default(); + let request = test_request( + SandboxPermissions::UseDefault, + ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + }, + ); + let runtime = UnifiedExecRuntime::new(&manager, zsh_fork_mode()); + + assert_eq!( + runtime.exec_approval_requirement(&request), + Some(ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }) + ); + } + + fn test_request( + sandbox_permissions: SandboxPermissions, + exec_approval_requirement: ExecApprovalRequirement, + ) -> UnifiedExecRequest { + let cwd = AbsolutePathBuf::try_from(std::env::current_dir().unwrap()) + .expect("current dir is absolute"); + UnifiedExecRequest { + command: vec!["zsh".to_string(), "-c".to_string(), "echo hi".to_string()], + shell_type: ShellType::Zsh, + hook_command: "echo hi".to_string(), + process_id: 1000, + cwd: cwd.clone(), + sandbox_cwd: cwd, + environment: Arc::new(Environment::default_for_tests()), + env: HashMap::new(), + exec_server_env_config: None, + explicit_env_overrides: HashMap::new(), + network: None, + tty: false, + sandbox_permissions, + additional_permissions: None, + #[cfg(unix)] + additional_permissions_preapproved: false, + justification: None, + exec_approval_requirement, + } + } + + fn zsh_fork_mode() -> UnifiedExecShellMode { + let cwd = std::env::current_dir().expect("read current dir"); + UnifiedExecShellMode::ZshFork(ZshForkConfig { + shell_zsh_path: AbsolutePathBuf::try_from(cwd.join("zsh")).expect("absolute zsh path"), + main_execve_wrapper_exe: AbsolutePathBuf::try_from(cwd.join("execve-wrapper")) + .expect("absolute wrapper path"), + }) + } } diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index facef69cbb..0386907101 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -73,6 +73,7 @@ use codex_tools::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolOutput; use codex_tools::ToolSpec; +use codex_tools::UnifiedExecShellMode; use codex_tools::can_request_original_image_detail; use codex_tools::collect_code_mode_exec_prompt_tool_definitions; use codex_tools::collect_request_plugin_install_entries; @@ -564,6 +565,7 @@ fn add_shell_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut Planne allow_login_shell, exec_permission_approvals_enabled, include_environment_id, + include_shell_parameter: unified_exec_should_include_shell_parameter(turn_context), })); planned_tools.add(WriteStdinHandler); @@ -580,6 +582,17 @@ fn add_shell_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut Planne } } +fn unified_exec_should_include_shell_parameter(turn_context: &TurnContext) -> bool { + !matches!( + &turn_context.unified_exec_shell_mode, + UnifiedExecShellMode::ZshFork(_) + ) || turn_context + .environments + .turn_environments + .iter() + .any(|environment| environment.environment.is_remote()) +} + fn add_mcp_resource_tools(context: &CoreToolPlanContext<'_>, planned_tools: &mut PlannedTools) { if context.mcp_tools.is_some() { planned_tools.add(ListMcpResourcesHandler); diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index e2b14b5b14..ae2cdf1265 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -233,6 +233,21 @@ fn set_features(turn: &mut TurnContext, features: &[Feature]) { } } +fn zsh_fork_config_for_spec_plan_tests() -> codex_tools::ZshForkConfig { + let placeholder_exe = codex_utils_absolute_path::AbsolutePathBuf::try_from( + std::env::current_exe().expect("current exe path"), + ) + .expect("current exe should be absolute"); + + // Spec planning only checks whether the shell mode is ZshFork. These paths + // are never executed, so use a stable absolute placeholder instead of + // depending on packaged zsh-fork artifacts in schema tests. + codex_tools::ZshForkConfig { + shell_zsh_path: placeholder_exe.clone(), + main_execve_wrapper_exe: placeholder_exe, + } +} + fn update_config(turn: &mut TurnContext, update: impl FnOnce(&mut crate::config::Config)) { let mut config = (*turn.config).clone(); update(&mut config); @@ -406,6 +421,120 @@ async fn shell_family_registers_visible_unified_exec_and_hidden_legacy_shell() { plan.assert_visible_lacks(&["shell_command"]); plan.assert_registered_contains(&["exec_command", "write_stdin", "shell_command"]); assert_eq!(plan.exposure("shell_command"), ToolExposure::Hidden); + assert!(has_parameter(plan.visible_spec("exec_command"), "shell")); +} + +#[tokio::test] +async fn shell_zsh_fork_stays_standalone_until_unified_exec_composition_is_enabled() { + let standalone = probe(|turn| { + set_features(turn, &[Feature::ShellTool, Feature::UnifiedExec]); + set_feature(turn, Feature::ShellZshFork, /*enabled*/ true); + set_feature(turn, Feature::UnifiedExecZshFork, /*enabled*/ false); + turn.model_info.shell_type = ConfigShellToolType::ShellCommand; + }) + .await; + + standalone.assert_visible_contains(&["shell_command"]); + standalone.assert_visible_lacks(&["exec_command", "write_stdin"]); + standalone.assert_registered_contains(&["shell_command"]); + standalone.assert_registered_lacks(&["exec_command", "write_stdin"]); + + let composed = probe(|turn| { + set_features( + turn, + &[ + Feature::ShellTool, + Feature::UnifiedExec, + Feature::ShellZshFork, + Feature::UnifiedExecZshFork, + ], + ); + turn.model_info.shell_type = ConfigShellToolType::ShellCommand; + }) + .await; + + if codex_utils_pty::conpty_supported() { + composed.assert_visible_contains(&["exec_command", "write_stdin"]); + composed.assert_visible_lacks(&["shell_command"]); + composed.assert_registered_contains(&["exec_command", "write_stdin", "shell_command"]); + assert_eq!(composed.exposure("shell_command"), ToolExposure::Hidden); + } else { + composed.assert_visible_contains(&["shell_command"]); + composed.assert_visible_lacks(&["exec_command", "write_stdin"]); + } +} + +#[tokio::test] +async fn zsh_fork_unified_exec_hides_shell_parameter() { + if !codex_utils_pty::conpty_supported() { + return; + } + + let plan = probe(|turn| { + set_features( + turn, + &[ + Feature::ShellTool, + Feature::UnifiedExec, + Feature::ShellZshFork, + Feature::UnifiedExecZshFork, + ], + ); + turn.unified_exec_shell_mode = + codex_tools::UnifiedExecShellMode::ZshFork(zsh_fork_config_for_spec_plan_tests()); + }) + .await; + + plan.assert_visible_contains(&["exec_command", "write_stdin"]); + assert!(!has_parameter(plan.visible_spec("exec_command"), "shell")); +} + +#[tokio::test] +async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_available() { + if !codex_utils_pty::conpty_supported() { + return; + } + + let plan = probe(|turn| { + set_features( + turn, + &[ + Feature::ShellTool, + Feature::UnifiedExec, + Feature::ShellZshFork, + Feature::UnifiedExecZshFork, + ], + ); + turn.unified_exec_shell_mode = + codex_tools::UnifiedExecShellMode::ZshFork(zsh_fork_config_for_spec_plan_tests()); + let remote_cwd = turn + .environments + .primary() + .expect("primary environment") + .cwd + .clone(); + turn.environments + .turn_environments + .push(crate::session::turn_context::TurnEnvironment { + environment_id: "remote".to_string(), + environment: Arc::new( + codex_exec_server::Environment::create_for_tests(Some( + "ws://127.0.0.1:1/remote-exec-server".to_string(), + )) + .expect("remote test environment"), + ), + cwd: remote_cwd, + shell: None, + }); + }) + .await; + + plan.assert_visible_contains(&["exec_command", "write_stdin"]); + assert!(has_parameter(plan.visible_spec("exec_command"), "shell")); + assert!(has_parameter( + plan.visible_spec("exec_command"), + "environment_id" + )); } #[tokio::test] diff --git a/codex-rs/core/src/turn_metadata.rs b/codex-rs/core/src/turn_metadata.rs index af4d23a7c1..7296d6d3f0 100644 --- a/codex-rs/core/src/turn_metadata.rs +++ b/codex-rs/core/src/turn_metadata.rs @@ -261,6 +261,7 @@ impl TurnMetadataState { session_id: String, thread_id: String, forked_from_thread_id: Option, + parent_thread_id: Option, session_source: &SessionSource, thread_source: Option, turn_id: String, @@ -278,18 +279,15 @@ impl TurnMetadataState { ) .to_string(), ); - let (parent_thread_id, subagent_kind) = match session_source { - SessionSource::SubAgent(subagent_source) => ( - subagent_source.parent_thread_id().or(forked_from_thread_id), - Some(subagent_source.kind().to_string()), - ), + let subagent_kind = match session_source { + SessionSource::SubAgent(subagent_source) => Some(subagent_source.kind().to_string()), SessionSource::Cli | SessionSource::VSCode | SessionSource::Exec | SessionSource::Mcp | SessionSource::Custom(_) | SessionSource::Internal(_) - | SessionSource::Unknown => (None, None), + | SessionSource::Unknown => None, }; let base_metadata = TurnMetadataBag { request_kind: None, diff --git a/codex-rs/core/src/turn_metadata_tests.rs b/codex-rs/core/src/turn_metadata_tests.rs index f6338f1c63..ffbeb83d26 100644 --- a/codex-rs/core/src/turn_metadata_tests.rs +++ b/codex-rs/core/src/turn_metadata_tests.rs @@ -122,6 +122,7 @@ fn turn_metadata_state_uses_platform_sandbox_tag() { "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, &SessionSource::Exec, Some(ThreadSource::User), "turn-a".to_string(), @@ -163,6 +164,7 @@ fn turn_metadata_state_uses_explicit_subagent_thread_source() { "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, &SessionSource::Exec, Some(ThreadSource::Subagent), "turn-a".to_string(), @@ -191,6 +193,7 @@ fn turn_metadata_state_includes_root_fork_lineage() { "session-a".to_string(), "thread-a".to_string(), Some(source_thread_id), + /*parent_thread_id*/ None, &SessionSource::Exec, Some(ThreadSource::User), "turn-a".to_string(), @@ -223,6 +226,7 @@ fn turn_metadata_state_includes_thread_spawn_subagent_parent_without_fork() { "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + Some(parent_thread_id), &SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, depth: 1, @@ -261,6 +265,7 @@ fn turn_metadata_state_includes_forked_thread_spawn_subagent_lineage() { "session-a".to_string(), "thread-a".to_string(), Some(parent_thread_id), + Some(parent_thread_id), &SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, depth: 1, @@ -291,38 +296,46 @@ fn turn_metadata_state_includes_forked_thread_spawn_subagent_lineage() { } #[test] -fn turn_metadata_state_includes_known_parent_for_other_subagent() { +fn turn_metadata_state_includes_known_parent_for_non_thread_spawn_subagents_without_fork() { let temp_dir = TempDir::new().expect("temp dir"); let cwd = temp_dir.path().abs(); let permission_profile = PermissionProfile::read_only(); let parent_thread_id = ThreadId::from_string("44444444-4444-4444-8444-444444444444").expect("thread id"); + let sources = [ + (SubAgentSource::Review, "review"), + (SubAgentSource::Other("guardian".to_string()), "guardian"), + ( + SubAgentSource::Other("agent_job:job-1".to_string()), + "agent_job:job-1", + ), + ]; - let state = TurnMetadataState::new( - "session-a".to_string(), - "thread-a".to_string(), - Some(parent_thread_id), - &SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())), - Some(ThreadSource::Subagent), - "turn-a".to_string(), - cwd, - &permission_profile, - WindowsSandboxLevel::Disabled, - /*enforce_managed_network*/ false, - ); + for (subagent_source, subagent_kind) in sources { + let state = TurnMetadataState::new( + "session-a".to_string(), + "thread-a".to_string(), + /*forked_from_thread_id*/ None, + Some(parent_thread_id), + &SessionSource::SubAgent(subagent_source), + Some(ThreadSource::Subagent), + "turn-a".to_string(), + cwd.clone(), + &permission_profile, + WindowsSandboxLevel::Disabled, + /*enforce_managed_network*/ false, + ); - let header = state.current_header_value().expect("header"); - let json: Value = serde_json::from_str(&header).expect("json"); + let header = state.current_header_value().expect("header"); + let json: Value = serde_json::from_str(&header).expect("json"); - assert_eq!( - json["forked_from_thread_id"].as_str(), - Some("44444444-4444-4444-8444-444444444444") - ); - assert_eq!( - json["parent_thread_id"].as_str(), - Some("44444444-4444-4444-8444-444444444444") - ); - assert_eq!(json["subagent_kind"].as_str(), Some("guardian")); + assert!(json.get("forked_from_thread_id").is_none()); + assert_eq!( + json["parent_thread_id"].as_str(), + Some("44444444-4444-4444-8444-444444444444") + ); + assert_eq!(json["subagent_kind"].as_str(), Some(subagent_kind)); + } } #[test] @@ -335,6 +348,7 @@ fn turn_metadata_state_includes_turn_started_at_unix_ms_after_start() { "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, &SessionSource::Exec, Some(ThreadSource::User), "turn-a".to_string(), @@ -364,6 +378,7 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta( "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, &SessionSource::Exec, /*thread_source*/ None, "turn-a".to_string(), @@ -412,6 +427,7 @@ fn turn_metadata_state_marks_user_input_requested_during_turn_only_for_mcp_reque "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, &SessionSource::Exec, /*thread_source*/ None, "turn-a".to_string(), @@ -464,6 +480,7 @@ fn turn_metadata_state_ignores_client_reserved_metadata_before_start() { "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, &SessionSource::Exec, Some(ThreadSource::User), "turn-a".to_string(), @@ -511,6 +528,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields( "session-a".to_string(), "thread-a".to_string(), Some(source_thread_id), + Some(parent_thread_id), &SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, depth: 1, @@ -612,6 +630,7 @@ fn turn_metadata_state_overlays_compaction_only_on_compaction_requests() { "session-a".to_string(), "thread-a".to_string(), /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, &SessionSource::Exec, Some(ThreadSource::User), "turn-a".to_string(), @@ -671,6 +690,7 @@ async fn turn_metadata_state_preserves_lineage_after_git_enrichment() { "session-a".to_string(), "thread-a".to_string(), Some(parent_thread_id), + Some(parent_thread_id), &SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, depth: 1, diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 6be32d26e7..94a7ab7c73 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -30,6 +30,7 @@ use std::sync::Weak; use codex_exec_server::Environment; use codex_network_proxy::NetworkProxy; use codex_protocol::models::AdditionalPermissionProfile; +use codex_tools::UnifiedExecShellMode; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::TruncationPolicy; use rand::Rng; @@ -97,6 +98,7 @@ pub(crate) struct ExecCommandRequest { pub cwd: AbsolutePathBuf, pub sandbox_cwd: AbsolutePathBuf, pub environment: Arc, + pub shell_mode: UnifiedExecShellMode, pub network: Option, pub tty: bool, pub sandbox_permissions: SandboxPermissions, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 83c750ebc4..563d486c69 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -1010,8 +1010,7 @@ impl UnifiedExecProcessManager { local_policy_env, }; let mut orchestrator = ToolOrchestrator::new(); - let mut runtime = - UnifiedExecRuntime::new(self, context.turn.unified_exec_shell_mode.clone()); + let mut runtime = UnifiedExecRuntime::new(self, request.shell_mode.clone()); let file_system_sandbox_policy = context.turn.file_system_sandbox_policy(); let exec_approval_requirement = context .session diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index 4d0597a844..9aa0e73814 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -179,6 +179,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() { .environments .primary_environment() .expect("primary environment"), + shell_mode: codex_tools::UnifiedExecShellMode::Direct, network: None, tty: true, sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, diff --git a/codex-rs/core/tests/common/zsh_fork.rs b/codex-rs/core/tests/common/zsh_fork.rs index e58ebd81dd..5abfc753b9 100644 --- a/codex-rs/core/tests/common/zsh_fork.rs +++ b/codex-rs/core/tests/common/zsh_fork.rs @@ -94,6 +94,33 @@ where builder.build(server).await } +pub async fn build_unified_exec_zsh_fork_test( + server: &wiremock::MockServer, + runtime: ZshForkRuntime, + approval_policy: AskForApproval, + permission_profile: PermissionProfile, + pre_build_hook: F, +) -> Result +where + F: FnOnce(&Path) + Send + 'static, +{ + let mut builder = test_codex() + .with_pre_build_hook(pre_build_hook) + .with_config(move |config| { + runtime.apply_to_config(config, approval_policy, permission_profile); + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::UnifiedExecZshFork) + .expect("test config should allow feature update"); + }); + builder.build(server).await +} + fn find_test_zsh_path() -> Result> { let repo_root = codex_utils_cargo_bin::repo_root()?; let dotslash_zsh = repo_root.join("codex-rs/app-server/tests/suite/zsh"); diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 6f0429e644..eda48a2c0e 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -105,6 +105,7 @@ async fn responses_stream_includes_subagent_header_on_review() { /*installation_id*/ TEST_INSTALLATION_ID.to_string(), provider.clone(), session_source, + /*parent_thread_id*/ None, config.model_verbosity, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, @@ -233,6 +234,7 @@ async fn responses_stream_includes_subagent_header_on_other() { /*installation_id*/ TEST_INSTALLATION_ID.to_string(), provider.clone(), session_source, + /*parent_thread_id*/ None, config.model_verbosity, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, @@ -350,6 +352,7 @@ async fn responses_respects_model_info_overrides_from_config() { /*installation_id*/ TEST_INSTALLATION_ID.to_string(), provider.clone(), session_source, + /*parent_thread_id*/ None, config.model_verbosity, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index c7233c6185..b191050d71 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -45,6 +45,7 @@ use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_with_timeout; +use core_test_support::zsh_fork::build_unified_exec_zsh_fork_test; use core_test_support::zsh_fork::build_zsh_fork_test; use core_test_support::zsh_fork::restrictive_workspace_write_profile; use core_test_support::zsh_fork::zsh_fork_runtime; @@ -2798,6 +2799,122 @@ async fn matched_prefix_rule_runs_unsandboxed_under_zsh_fork() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[cfg(unix)] +async fn unified_exec_zsh_fork_parent_approval_escalates_intercepted_exec() -> Result<()> { + skip_if_no_network!(Ok(())); + + let Some(runtime) = zsh_fork_runtime("unified-exec zsh-fork parent approval test")? else { + return Ok(()); + }; + + let approval_policy = AskForApproval::OnRequest; + let permission_profile = restrictive_workspace_write_profile(); + let outside_dir = tempfile::tempdir_in(std::env::current_dir()?)?; + let outside_path = outside_dir + .path() + .join("unified-exec-zsh-fork-parent-approval.txt"); + let command = format!("touch {outside_path:?}"); + + let server = start_mock_server().await; + let outside_path_for_hook = outside_path.clone(); + let test = build_unified_exec_zsh_fork_test( + &server, + runtime, + approval_policy, + permission_profile, + move |_home| { + let _ = fs::remove_file(&outside_path_for_hook); + }, + ) + .await?; + + let call_id = "uexec-zsh-fork-parent-approval"; + let event = exec_command_event( + call_id, + &command, + Some(30_000), + SandboxPermissions::RequireEscalated, + Some("write outside the workspace for the test"), + )?; + let _ = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-uexec-zsh-fork-parent-approval-1"), + event, + ev_completed("resp-uexec-zsh-fork-parent-approval-1"), + ]), + ) + .await; + let results = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-uexec-zsh-fork-parent-approval-1", "done"), + ev_completed("resp-uexec-zsh-fork-parent-approval-2"), + ]), + ) + .await; + + let session_model = test.session_configured.model.clone(); + let (sandbox_policy, permission_profile) = turn_permission_fields( + test.session_configured.permission_profile.clone(), + test.cwd.path(), + ); + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "run approved unified exec through zsh fork".into(), + text_elements: Vec::new(), + }], + environments: None, + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + cwd: Some(test.cwd.path().to_path_buf()), + approval_policy: Some(approval_policy), + approvals_reviewer: Some(ApprovalsReviewer::User), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: session_model, + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + let approval = expect_exec_approval(&test, &command).await; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + wait_for_completion_without_approval(&test).await; + + let result = parse_result(&results.single_request().function_call_output(call_id)); + assert_eq!( + result.exit_code.unwrap_or(0), + 0, + "approved unified exec zsh-fork command should complete: {}", + result.stdout + ); + assert!( + outside_path.exists(), + "expected parent approval to let the intercepted touch run unsandboxed; output: {}", + result.stdout + ); + + Ok(()) +} + #[tokio::test(flavor = "current_thread")] #[cfg(unix)] async fn invalid_requested_prefix_rule_falls_back_for_compound_command() -> Result<()> { diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index c77bd4daad..560761551c 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -489,6 +489,7 @@ async fn resume_replays_legacy_js_repl_image_rollout_shapes() { item: RolloutItem::SessionMeta(SessionMetaLine { meta: SessionMeta { id: ThreadId::default(), + parent_thread_id: None, timestamp: "2024-01-01T00:00:00Z".to_string(), cwd: ".".into(), originator: "test_originator".to_string(), @@ -619,6 +620,7 @@ async fn resume_replays_image_tool_outputs_with_detail() { item: RolloutItem::SessionMeta(SessionMetaLine { meta: SessionMeta { id: ThreadId::default(), + parent_thread_id: None, timestamp: "2024-01-01T00:00:00Z".to_string(), cwd: ".".into(), originator: "test_originator".to_string(), @@ -903,6 +905,7 @@ async fn send_provider_auth_request(server: &MockServer, auth: ModelProviderAuth /*installation_id*/ "11111111-1111-4111-8111-111111111111".to_string(), provider, SessionSource::Exec, + /*parent_thread_id*/ None, config.model_verbosity, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, @@ -2359,6 +2362,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { /*installation_id*/ "11111111-1111-4111-8111-111111111111".to_string(), provider.clone(), SessionSource::Exec, + /*parent_thread_id*/ None, config.model_verbosity, /*enable_request_compression*/ false, /*include_timing_metrics*/ false, diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 09d0093f99..d9e94204f2 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -2155,6 +2155,7 @@ async fn websocket_harness_with_provider_options( /*installation_id*/ TEST_INSTALLATION_ID.to_string(), provider.clone(), SessionSource::Exec, + /*parent_thread_id*/ None, config.model_verbosity, /*enable_request_compression*/ false, runtime_metrics_enabled, diff --git a/codex-rs/core/tests/suite/personality_migration.rs b/codex-rs/core/tests/suite/personality_migration.rs index 9cee89d761..84a2e18824 100644 --- a/codex-rs/core/tests/suite/personality_migration.rs +++ b/codex-rs/core/tests/suite/personality_migration.rs @@ -61,6 +61,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R meta: SessionMeta { id: thread_id, forked_from_id: None, + parent_thread_id: None, timestamp: TEST_TIMESTAMP.to_string(), cwd: std::path::PathBuf::from("."), originator: "test_originator".to_string(), @@ -109,6 +110,7 @@ async fn write_rollout_with_meta_only(dir: &Path, thread_id: ThreadId) -> io::Re meta: SessionMeta { id: thread_id, forked_from_id: None, + parent_thread_id: None, timestamp: TEST_TIMESTAMP.to_string(), cwd: std::path::PathBuf::from("."), originator: "test_originator".to_string(), diff --git a/codex-rs/core/tests/suite/responses_api_proxy_headers.rs b/codex-rs/core/tests/suite/responses_api_proxy_headers.rs index 0c401d05fc..144feb016f 100644 --- a/codex-rs/core/tests/suite/responses_api_proxy_headers.rs +++ b/codex-rs/core/tests/suite/responses_api_proxy_headers.rs @@ -130,8 +130,9 @@ async fn responses_api_parent_and_subagent_requests_include_identity_headers() - .header("x-codex-turn-metadata") .ok_or_else(|| anyhow!("child request missing x-codex-turn-metadata"))?, )?; + assert!(child_turn_metadata.get("forked_from_thread_id").is_none()); assert_eq!( - child_turn_metadata["forked_from_thread_id"].as_str(), + child_turn_metadata["parent_thread_id"].as_str(), Some(parent_thread_id) ); diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index 9efaf6c2d1..0d3f564893 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -136,8 +136,9 @@ async fn review_op_emits_lifecycle_and_review_output() { .expect("review request turn metadata"), ) .expect("review request turn metadata json"); + assert!(turn_metadata.get("forked_from_thread_id").is_none()); assert_eq!( - turn_metadata["forked_from_thread_id"].as_str(), + turn_metadata["parent_thread_id"].as_str(), Some(parent_thread_id.as_str()) ); diff --git a/codex-rs/core/tests/suite/rollout_list_find.rs b/codex-rs/core/tests/suite/rollout_list_find.rs index 48a648d1bc..4ff35449cb 100644 --- a/codex-rs/core/tests/suite/rollout_list_find.rs +++ b/codex-rs/core/tests/suite/rollout_list_find.rs @@ -184,6 +184,7 @@ async fn find_locates_rollout_file_written_by_recorder() -> std::io::Result<()> RolloutRecorderParams::new( thread_id, /*forked_from_id*/ None, + /*parent_thread_id*/ None, SessionSource::Exec, /*thread_source*/ None, BaseInstructions::default(), diff --git a/codex-rs/core/tests/suite/spawn_agent_description.rs b/codex-rs/core/tests/suite/spawn_agent_description.rs index f9be02349c..895ef85453 100644 --- a/codex-rs/core/tests/suite/spawn_agent_description.rs +++ b/codex-rs/core/tests/suite/spawn_agent_description.rs @@ -184,7 +184,7 @@ async fn spawn_agent_description_lists_visible_models_and_reasoning_efforts() -> ); assert!( description.contains( - "Spawned agents inherit your current model by default. If provided, `model` specifies the model to use for the spawned agent." + "Spawned agents inherit your current model by default. Omit `model` to use that preferred default; set `model` only when an explicit override is needed." ), "expected inherited-model guidance in spawn_agent description: {description:?}" ); diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 4d9c89638e..aecb9a392e 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -212,6 +212,7 @@ async fn backfill_scans_existing_rollouts() -> Result<()> { meta: SessionMeta { id: thread_id, forked_from_id: None, + parent_thread_id: None, timestamp: "2026-01-27T12:00:00Z".to_string(), cwd: codex_home.to_path_buf(), originator: "test".to_string(), diff --git a/codex-rs/exec/src/event_processor_with_human_output_tests.rs b/codex-rs/exec/src/event_processor_with_human_output_tests.rs index 1e900f2b61..5e89d48c7f 100644 --- a/codex-rs/exec/src/event_processor_with_human_output_tests.rs +++ b/codex-rs/exec/src/event_processor_with_human_output_tests.rs @@ -211,6 +211,7 @@ async fn config_summary_entries_include_runtime_workspace_roots() { session_id: SessionId::new(), thread_id: ThreadId::new(), forked_from_id: None, + parent_thread_id: None, thread_source: None, thread_name: None, model: "gpt-5.4".to_string(), diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 8d632396b0..69ad574666 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -1070,6 +1070,7 @@ fn session_configured_from_thread_start_response( session_configured_from_thread_response( &response.thread.session_id, &response.thread.id, + response.thread.parent_thread_id.as_deref(), response.thread.thread_source.map(Into::into), response.thread.name.clone(), response.thread.path.clone(), @@ -1092,6 +1093,7 @@ fn session_configured_from_thread_resume_response( session_configured_from_thread_response( &response.thread.session_id, &response.thread.id, + response.thread.parent_thread_id.as_deref(), response.thread.thread_source.map(Into::into), response.thread.name.clone(), response.thread.path.clone(), @@ -1123,6 +1125,7 @@ fn review_target_to_api(target: ReviewTarget) -> ApiReviewTarget { fn session_configured_from_thread_response( session_id: &str, thread_id: &str, + parent_thread_id: Option<&str>, thread_source: Option, thread_name: Option, rollout_path: Option, @@ -1140,11 +1143,16 @@ fn session_configured_from_thread_response( .map_err(|err| format!("session id `{session_id}` is invalid: {err}"))?; let thread_id = ThreadId::from_string(thread_id) .map_err(|err| format!("thread id `{thread_id}` is invalid: {err}"))?; + let parent_thread_id = parent_thread_id + .map(ThreadId::from_string) + .transpose() + .map_err(|err| format!("parent thread id is invalid: {err}"))?; Ok(SessionConfiguredEvent { session_id, thread_id, forked_from_id: None, + parent_thread_id, thread_source, thread_name, model, diff --git a/codex-rs/exec/src/lib_tests.rs b/codex-rs/exec/src/lib_tests.rs index 79d07a0b76..8662c2cfeb 100644 --- a/codex-rs/exec/src/lib_tests.rs +++ b/codex-rs/exec/src/lib_tests.rs @@ -307,6 +307,7 @@ fn turn_items_for_thread_returns_matching_turn_items() { id: "thread-1".to_string(), session_id: "thread-1".to_string(), forked_from_id: None, + parent_thread_id: None, preview: String::new(), ephemeral: false, model_provider: "openai".to_string(), @@ -585,12 +586,33 @@ async fn session_configured_from_thread_response_preserves_thread_source() { ); } +#[tokio::test] +async fn session_configured_from_thread_response_preserves_parent_thread_id() { + let codex_home = tempdir().expect("create temp codex home"); + let cwd = tempdir().expect("create temp cwd"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(cwd.path().to_path_buf())) + .build() + .await + .expect("build config"); + let parent_thread_id = ThreadId::new(); + let mut response = sample_thread_start_response(); + response.thread.parent_thread_id = Some(parent_thread_id.to_string()); + + let event = session_configured_from_thread_start_response(&response, &config) + .expect("build bootstrap session configured event"); + + assert_eq!(event.parent_thread_id, Some(parent_thread_id)); +} + fn sample_thread_start_response() -> ThreadStartResponse { ThreadStartResponse { thread: codex_app_server_protocol::Thread { id: "67e55044-10b1-426f-9247-bb680e5fe0c8".to_string(), session_id: "67e55044-10b1-426f-9247-bb680e5fe0c7".to_string(), forked_from_id: None, + parent_thread_id: None, preview: String::new(), ephemeral: false, model_provider: "openai".to_string(), 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 ff97b6832c..2cee9b0e83 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -111,6 +111,7 @@ fn session_configured_produces_thread_started_event() { session_id: SessionId::from(thread_id), thread_id, forked_from_id: None, + parent_thread_id: None, thread_source: None, thread_name: None, model: "codex-mini-latest".to_string(), diff --git a/codex-rs/ext/goal/BUILD.bazel b/codex-rs/ext/goal/BUILD.bazel index 05b80c3706..e13276ddd5 100644 --- a/codex-rs/ext/goal/BUILD.bazel +++ b/codex-rs/ext/goal/BUILD.bazel @@ -3,5 +3,8 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "goal", crate_name = "codex_goal_extension", + compile_data = glob([ + "templates/**", + ]), integration_compile_data_extra = ["src/accounting.rs"], ) diff --git a/codex-rs/ext/goal/Cargo.toml b/codex-rs/ext/goal/Cargo.toml index 3bf0eca8d5..667ed5d14d 100644 --- a/codex-rs/ext/goal/Cargo.toml +++ b/codex-rs/ext/goal/Cargo.toml @@ -21,6 +21,7 @@ codex-otel = { workspace = true } codex-protocol = { workspace = true } codex-state = { workspace = true } codex-tools = { workspace = true } +codex-utils-template = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } tracing = { workspace = true } diff --git a/codex-rs/ext/goal/src/api.rs b/codex-rs/ext/goal/src/api.rs new file mode 100644 index 0000000000..0e23c295a2 --- /dev/null +++ b/codex-rs/ext/goal/src/api.rs @@ -0,0 +1,286 @@ +use std::collections::HashMap; +use std::fmt; +use std::sync::Arc; +use std::sync::Mutex; +use std::sync::PoisonError; +use std::sync::Weak; + +use codex_protocol::ThreadId; +use codex_protocol::protocol::ThreadGoal; +use codex_protocol::protocol::ThreadGoalStatus; +use codex_protocol::protocol::validate_thread_goal_objective; + +use crate::runtime::GoalRuntimeHandle; +use crate::runtime::PreviousGoalSnapshot; +use crate::tool::fill_empty_thread_preview_if_possible; +use crate::tool::protocol_goal_from_state; +use crate::tool::state_status_from_protocol; +use crate::tool::validate_goal_budget; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GoalServiceError { + InvalidRequest(String), + Internal(String), +} + +impl fmt::Display for GoalServiceError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidRequest(message) | Self::Internal(message) => f.write_str(message), + } + } +} + +impl std::error::Error for GoalServiceError {} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum GoalObjectiveUpdate<'a> { + Keep, + Set(&'a str), +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum GoalTokenBudgetUpdate { + Keep, + Set(Option), +} + +#[derive(Clone, Copy, Debug)] +pub struct GoalSetRequest<'a> { + pub thread_id: ThreadId, + pub objective: GoalObjectiveUpdate<'a>, + pub status: Option, + pub token_budget: GoalTokenBudgetUpdate, +} + +#[derive(Clone, Debug)] +pub struct GoalSetOutcome { + pub goal: ThreadGoal, + state_goal: codex_state::ThreadGoal, + previous_goal: Option, +} + +impl GoalSetOutcome { + pub async fn apply_runtime_effects(&self, goal_service: &GoalService) { + if let Some(runtime) = goal_service.runtime_for_thread(self.goal.thread_id) + && let Err(err) = runtime + .apply_external_goal_set(self.state_goal.clone(), self.previous_goal.clone()) + .await + { + tracing::warn!("failed to apply external goal status runtime effects: {err}"); + } + } +} + +#[derive(Debug, Default)] +pub struct GoalService { + runtimes: Mutex>>, +} + +impl GoalService { + pub fn new() -> Self { + Self::default() + } + + pub async fn get_thread_goal( + &self, + state_db: &codex_state::StateRuntime, + thread_id: ThreadId, + ) -> Result, GoalServiceError> { + state_db + .thread_goals() + .get_thread_goal(thread_id) + .await + .map(|goal| goal.map(protocol_goal_from_state)) + .map_err(|err| GoalServiceError::Internal(format!("failed to read thread goal: {err}"))) + } + + pub async fn set_thread_goal( + &self, + state_db: &codex_state::StateRuntime, + request: GoalSetRequest<'_>, + ) -> Result { + let GoalSetRequest { + thread_id, + objective, + status, + token_budget, + } = request; + let status = status.map(state_status_from_protocol); + let objective = match objective { + GoalObjectiveUpdate::Keep => None, + GoalObjectiveUpdate::Set(objective) => Some(objective.trim()), + }; + let token_budget = match token_budget { + GoalTokenBudgetUpdate::Keep => None, + GoalTokenBudgetUpdate::Set(token_budget) => Some(token_budget), + }; + + if let Some(objective) = objective { + validate_thread_goal_objective(objective).map_err(GoalServiceError::InvalidRequest)?; + } + if objective.is_some() || token_budget.is_some() { + validate_goal_budget(token_budget.flatten()) + .map_err(GoalServiceError::InvalidRequest)?; + } + + if let Some(runtime) = self.runtime_for_thread(thread_id) + && let Err(err) = runtime.prepare_external_goal_mutation().await + { + tracing::warn!("failed to prepare external goal mutation: {err}"); + } + + let (goal, previous_goal) = if let Some(objective) = objective { + let existing_goal = state_db + .thread_goals() + .get_thread_goal(thread_id) + .await + .map_err(|err| { + GoalServiceError::Internal(format!("failed to read thread goal: {err}")) + })?; + if let Some(existing_goal) = existing_goal.as_ref() { + let previous_goal = PreviousGoalSnapshot::from(existing_goal); + state_db + .thread_goals() + .update_thread_goal( + thread_id, + codex_state::GoalUpdate { + objective: Some(objective.to_string()), + status, + token_budget, + expected_goal_id: Some(existing_goal.goal_id.clone()), + }, + ) + .await + .map_err(|err| { + GoalServiceError::Internal(format!("failed to update thread goal: {err}")) + })? + .ok_or_else(|| { + GoalServiceError::InvalidRequest(format!( + "cannot update goal for thread {thread_id}: no goal exists" + )) + }) + .map(|goal| (goal, Some(previous_goal)))? + } else { + state_db + .thread_goals() + .replace_thread_goal( + thread_id, + objective, + status.unwrap_or(codex_state::ThreadGoalStatus::Active), + token_budget.flatten(), + ) + .await + .map_err(|err| { + GoalServiceError::Internal(format!("failed to replace thread goal: {err}")) + }) + .map(|goal| (goal, None))? + } + } else { + let existing_goal = state_db + .thread_goals() + .get_thread_goal(thread_id) + .await + .map_err(|err| { + GoalServiceError::Internal(format!("failed to read thread goal: {err}")) + })? + .ok_or_else(|| { + GoalServiceError::InvalidRequest(format!( + "cannot update goal for thread {thread_id}: no goal exists" + )) + })?; + let previous_goal = PreviousGoalSnapshot::from(&existing_goal); + let expected_goal_id = existing_goal.goal_id.clone(); + state_db + .thread_goals() + .update_thread_goal( + thread_id, + codex_state::GoalUpdate { + objective: None, + status, + token_budget, + expected_goal_id: Some(expected_goal_id), + }, + ) + .await + .map_err(|err| { + GoalServiceError::Internal(format!("failed to update thread goal: {err}")) + })? + .ok_or_else(|| { + GoalServiceError::InvalidRequest(format!( + "cannot update goal for thread {thread_id}: no goal exists" + )) + }) + .map(|goal| (goal, Some(previous_goal)))? + }; + + if objective.is_some() { + fill_empty_thread_preview_if_possible(state_db, thread_id, &goal).await; + } + Ok(GoalSetOutcome { + goal: protocol_goal_from_state(goal.clone()), + state_goal: goal, + previous_goal, + }) + } + + pub async fn clear_thread_goal( + &self, + state_db: &codex_state::StateRuntime, + thread_id: ThreadId, + ) -> Result { + if let Some(runtime) = self.runtime_for_thread(thread_id) + && let Err(err) = runtime.prepare_external_goal_mutation().await + { + tracing::warn!("failed to prepare external goal mutation: {err}"); + } + + let cleared = state_db + .thread_goals() + .delete_thread_goal(thread_id) + .await + .map_err(|err| { + GoalServiceError::Internal(format!("failed to clear thread goal: {err}")) + })?; + + if cleared + && let Some(runtime) = self.runtime_for_thread(thread_id) + && let Err(err) = runtime.apply_external_goal_clear().await + { + tracing::warn!("failed to apply external goal clear runtime effects: {err}"); + } + + Ok(cleared) + } + + pub(crate) fn register_runtime(&self, runtime: &Arc) { + self.runtimes() + .insert(runtime.thread_id().to_string(), Arc::downgrade(runtime)); + } + + pub(crate) fn unregister_runtime(&self, runtime: &Arc) { + let key = runtime.thread_id().to_string(); + let runtime = Arc::downgrade(runtime); + let mut runtimes = self.runtimes(); + if runtimes + .get(&key) + .is_some_and(|registered| registered.ptr_eq(&runtime)) + { + runtimes.remove(&key); + } + } + + fn runtime_for_thread(&self, thread_id: ThreadId) -> Option> { + let key = thread_id.to_string(); + let mut runtimes = self.runtimes(); + let runtime = runtimes.get(&key).and_then(Weak::upgrade); + if runtime.is_none() { + runtimes.remove(&key); + } + runtime + } + + fn runtimes(&self) -> std::sync::MutexGuard<'_, HashMap>> { + self.runtimes.lock().unwrap_or_else(PoisonError::into_inner) + } +} diff --git a/codex-rs/ext/goal/src/extension.rs b/codex-rs/ext/goal/src/extension.rs index e09f041e31..563c40b503 100644 --- a/codex-rs/ext/goal/src/extension.rs +++ b/codex-rs/ext/goal/src/extension.rs @@ -7,9 +7,11 @@ use codex_extension_api::ConfigContributor; use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionEventSink; use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::ThreadIdleInput; use codex_extension_api::ThreadLifecycleContributor; use codex_extension_api::ThreadResumeInput; use codex_extension_api::ThreadStartInput; +use codex_extension_api::ThreadStopInput; use codex_extension_api::TokenUsageContributor; use codex_extension_api::ToolCallOutcome; use codex_extension_api::ToolContributor; @@ -31,6 +33,7 @@ use codex_protocol::protocol::TokenUsageInfo; use crate::accounting::BudgetLimitedGoalDisposition; use crate::accounting::GoalAccountingState; +use crate::api::GoalService; use crate::events::GoalEventEmitter; use crate::metrics::GoalMetrics; use crate::runtime::GoalRuntimeConfig; @@ -56,6 +59,7 @@ pub struct GoalExtension { event_emitter: GoalEventEmitter, metrics: GoalMetrics, thread_manager: Weak, + goal_service: Arc, goals_enabled: Arc bool + Send + Sync>, } @@ -71,6 +75,7 @@ impl GoalExtension { event_sink: Arc, metrics_client: Option, thread_manager: Weak, + goal_service: Arc, goals_enabled: impl Fn(&C) -> bool + Send + Sync + 'static, ) -> Self { Self { @@ -78,6 +83,7 @@ impl GoalExtension { event_emitter: GoalEventEmitter::new(event_sink), metrics: GoalMetrics::new(metrics_client), thread_manager, + goal_service, goals_enabled: Arc::new(goals_enabled), } } @@ -119,6 +125,7 @@ where ) }); runtime.set_enabled(enabled); + self.goal_service.register_runtime(&runtime); } async fn on_thread_resume(&self, input: ThreadResumeInput<'_>) { @@ -133,6 +140,25 @@ where ); } } + + async fn on_thread_idle(&self, input: ThreadIdleInput<'_>) { + let Some(runtime) = goal_runtime_handle(input.thread_store) else { + return; + }; + + if let Err(err) = runtime.continue_if_idle().await { + tracing::warn!( + "failed to continue active goal for idle thread {}: {err}", + runtime.thread_id() + ); + } + } + + async fn on_thread_stop(&self, input: ThreadStopInput<'_>) { + if let Some(runtime) = goal_runtime_handle(input.thread_store) { + self.goal_service.unregister_runtime(&runtime); + } + } } impl ConfigContributor for GoalExtension @@ -394,6 +420,7 @@ pub fn install_with_backend( state_dbs: Arc, metrics_client: Option, thread_manager: Weak, + goal_service: Arc, goals_enabled: impl Fn(&C) -> bool + Send + Sync + 'static, ) where C: Send + Sync + 'static, @@ -403,6 +430,7 @@ pub fn install_with_backend( registry.event_sink(), metrics_client, thread_manager, + Arc::clone(&goal_service), goals_enabled, )); registry.thread_lifecycle_contributor(extension.clone()); diff --git a/codex-rs/ext/goal/src/lib.rs b/codex-rs/ext/goal/src/lib.rs index 545d9e699a..8433e1f7df 100644 --- a/codex-rs/ext/goal/src/lib.rs +++ b/codex-rs/ext/goal/src/lib.rs @@ -5,6 +5,7 @@ //! accounting that can be represented with today's extension API. mod accounting; +mod api; mod events; mod extension; mod metrics; @@ -13,6 +14,12 @@ mod spec; mod steering; mod tool; +pub use api::GoalObjectiveUpdate; +pub use api::GoalService; +pub use api::GoalServiceError; +pub use api::GoalSetOutcome; +pub use api::GoalSetRequest; +pub use api::GoalTokenBudgetUpdate; pub use extension::GoalExtension; pub use extension::GoalExtensionConfig; pub use extension::install_with_backend; diff --git a/codex-rs/ext/goal/src/runtime.rs b/codex-rs/ext/goal/src/runtime.rs index f077ddc54c..8d79391c95 100644 --- a/codex-rs/ext/goal/src/runtime.rs +++ b/codex-rs/ext/goal/src/runtime.rs @@ -12,6 +12,7 @@ use crate::accounting::BudgetLimitedGoalDisposition; use crate::accounting::GoalAccountingState; use crate::events::GoalEventEmitter; use crate::metrics::GoalMetrics; +use crate::steering::continuation_steering_item; use crate::steering::objective_updated_steering_item; use crate::tool::protocol_goal_from_state; @@ -157,10 +158,8 @@ impl GoalRuntimeHandle { self.inner .metrics .record_terminal_if_status_changed(previous_status, &goal); - let should_steer_active_turn = previous_goal.as_ref().is_none_or(|previous_goal| { - previous_goal.goal_id != goal.goal_id - || previous_goal.status != codex_state::ThreadGoalStatus::Active - || previous_goal.objective != goal.objective + let objective_changed = previous_goal.as_ref().is_some_and(|previous_goal| { + !replaced_existing_goal && previous_goal.objective != goal.objective }); match goal.status { codex_state::ThreadGoalStatus::Active => { @@ -174,10 +173,11 @@ impl GoalRuntimeHandle { .accounting_state .mark_idle_goal_active(goal.goal_id.clone()); } - if should_steer_active_turn { + if objective_changed { let item = objective_updated_steering_item(&protocol_goal_from_state(goal)); self.inject_active_turn_steering(item).await; } + self.continue_if_idle().await?; } codex_state::ThreadGoalStatus::BudgetLimited => { if self.inner.accounting_state.current_turn_id().is_none() { @@ -275,6 +275,57 @@ impl GoalRuntimeHandle { Ok(()) } + pub(crate) async fn continue_if_idle(&self) -> Result<(), String> { + if !self.tools_visible() { + self.inner.accounting_state.clear_active_goal(); + return Ok(()); + } + + let Some(goal) = self + .inner + .state_dbs + .thread_goals() + .get_thread_goal(self.thread_id()) + .await + .map_err(|err| err.to_string())? + else { + self.inner.accounting_state.clear_active_goal(); + return Ok(()); + }; + if goal.status != codex_state::ThreadGoalStatus::Active { + self.inner.accounting_state.clear_active_goal(); + return Ok(()); + } + + let item = continuation_steering_item(&protocol_goal_from_state(goal)); + let Some(thread_manager) = self.inner.thread_manager.upgrade() else { + tracing::debug!("skipping goal continuation because thread manager is unavailable"); + return Ok(()); + }; + let Ok(thread) = thread_manager.get_thread(self.inner.thread_id).await else { + tracing::debug!("skipping goal continuation because live thread is unavailable"); + return Ok(()); + }; + + if thread.try_start_turn_if_idle(vec![item]).await.is_err() { + tracing::debug!("skipping goal continuation because the thread is no longer idle"); + } + + let current_turn_is_goal_active = self + .inner + .accounting_state + .current_turn_id() + .is_some_and(|turn_id| { + self.inner + .accounting_state + .turn_is_current_active_goal(turn_id.as_str()) + }); + if !current_turn_is_goal_active { + self.inner.accounting_state.clear_active_goal(); + } + Ok(()) + } + pub(crate) async fn inject_active_turn_steering(&self, item: ResponseItem) { let Some(thread_manager) = self.inner.thread_manager.upgrade() else { tracing::debug!("skipping goal steering because thread manager is unavailable"); diff --git a/codex-rs/ext/goal/src/steering.rs b/codex-rs/ext/goal/src/steering.rs index e540ee171d..6615b6edaa 100644 --- a/codex-rs/ext/goal/src/steering.rs +++ b/codex-rs/ext/goal/src/steering.rs @@ -3,6 +3,36 @@ use codex_core::context::InternalContextSource; use codex_core::context::InternalModelContextFragment; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::ThreadGoal; +use codex_utils_template::Template; +use std::sync::LazyLock; + +static CONTINUATION_PROMPT_TEMPLATE: LazyLock