diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index fcc35a60dd..33c74ac982 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -518,6 +518,13 @@ server_request_definitions! { response: v2::FileChangeRequestApprovalResponse, }, + /// Sent when approval is requested to install missing skill dependencies. + /// This request is used for Turns started via turn/start. + SkillDependenciesRequestApproval => "item/skillDependencies/requestApproval" { + params: v2::SkillDependenciesRequestApprovalParams, + response: v2::SkillDependenciesRequestApprovalResponse, + }, + /// EXPERIMENTAL - Request input from the user for a tool call. ToolRequestUserInput => "item/tool/requestUserInput" { params: v2::ToolRequestUserInputParams, diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 9eddf28148..9d6f875da6 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -548,6 +548,16 @@ pub enum FileChangeApprovalDecision { Cancel, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub enum SkillDependenciesApprovalDecision { + /// User approved installing missing dependencies. + Install, + /// User declined installation and chose to continue anyway. + RunAnyway, +} + #[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -2371,6 +2381,34 @@ pub struct FileChangeRequestApprovalResponse { pub decision: FileChangeApprovalDecision, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct SkillDependenciesRequestApprovalOption { + pub label: String, + pub description: String, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct SkillDependenciesRequestApprovalParams { + pub thread_id: String, + pub turn_id: String, + pub item_id: String, + pub header: String, + pub question: String, + pub run_anyway: SkillDependenciesRequestApprovalOption, + pub install: SkillDependenciesRequestApprovalOption, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct SkillDependenciesRequestApprovalResponse { + pub decision: SkillDependenciesApprovalDecision, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 659e2bf72c..f310253015 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -482,6 +482,17 @@ Order of messages: UI guidance for IDEs: surface an approval dialog as soon as the request arrives. The turn will proceed after the server receives a response to the approval request. The terminal `item/completed` notification will be sent with the appropriate status. +### Skill dependency approvals + +When a skill declares missing MCP dependencies, the server sends a request so the client can ask the user whether to install them. The client should present the prompt and reply with the user's decision. + +Order of messages: + +1. `item/skillDependencies/requestApproval` (request) - includes `threadId`, `turnId`, `itemId`, prompt text, and two options (`runAnyway`, `install`). +2. Client response - `{ "decision": "install" | "runAnyway" }`. + +After a response, the turn resumes. If the user chooses `install`, Codex will attempt to add the MCP server config before continuing. + ## Skills Invoke a skill by including `$` in the text input. Add a `skill` input item (recommended) so the backend injects full skill instructions instead of relying on the model to resolve the name. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 68e233b843..6fd1a2f217 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -49,6 +49,10 @@ use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification; use codex_app_server_protocol::ReasoningTextDeltaNotification; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; +use codex_app_server_protocol::SkillDependenciesApprovalDecision; +use codex_app_server_protocol::SkillDependenciesRequestApprovalOption; +use codex_app_server_protocol::SkillDependenciesRequestApprovalParams; +use codex_app_server_protocol::SkillDependenciesRequestApprovalResponse; use codex_app_server_protocol::TerminalInteractionNotification; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadRollbackResponse; @@ -80,6 +84,7 @@ use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; +use codex_core::protocol::SkillDependenciesApprovalRequestEvent; use codex_core::protocol::TokenCountEvent; use codex_core::protocol::TurnDiffEvent; use codex_core::review_format::format_review_findings_block; @@ -88,6 +93,7 @@ use codex_protocol::ThreadId; use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::ReviewOutputEvent; use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer; +use codex_protocol::request_user_input::RequestUserInputEvent; use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse; use std::collections::HashMap; use std::convert::TryFrom; @@ -269,8 +275,12 @@ pub(crate) async fn apply_bespoke_event_handling( }, EventMsg::RequestUserInput(request) => { if matches!(api_version, ApiVersion::V2) { - let questions = request - .questions + let RequestUserInputEvent { + call_id, + turn_id, + questions, + } = request; + let questions = questions .into_iter() .map(|question| ToolRequestUserInputQuestion { id: question.id, @@ -289,8 +299,8 @@ pub(crate) async fn apply_bespoke_event_handling( .collect(); let params = ToolRequestUserInputParams { thread_id: conversation_id.to_string(), - turn_id: request.turn_id, - item_id: request.call_id, + turn_id, + item_id: call_id, questions, }; let rx = outgoing @@ -318,6 +328,56 @@ pub(crate) async fn apply_bespoke_event_handling( } } } + EventMsg::SkillDependenciesApprovalRequest(request) => { + if matches!(api_version, ApiVersion::V2) { + let params = SkillDependenciesRequestApprovalParams { + thread_id: conversation_id.to_string(), + turn_id: request.turn_id.clone(), + item_id: request.call_id.clone(), + header: request.header.clone(), + question: request.question.clone(), + run_anyway: SkillDependenciesRequestApprovalOption { + label: request.run_anyway.label.clone(), + description: request.run_anyway.description.clone(), + }, + install: SkillDependenciesRequestApprovalOption { + label: request.install.label.clone(), + description: request.install.description.clone(), + }, + }; + let rx = outgoing + .send_request(ServerRequestPayload::SkillDependenciesRequestApproval( + params, + )) + .await; + tokio::spawn(async move { + on_skill_dependencies_request_approval_response( + event_turn_id, + rx, + conversation, + request, + ) + .await; + }); + } else { + error!( + "skill dependency approvals are only supported on api v2 (call_id: {})", + request.call_id + ); + let empty = CoreRequestUserInputResponse { + answers: HashMap::new(), + }; + if let Err(err) = conversation + .submit(Op::UserInputAnswer { + id: event_turn_id, + response: empty, + }) + .await + { + error!("failed to submit UserInputAnswer: {err}"); + } + } + } // TODO(celia): properly construct McpToolCall TurnItem in core. EventMsg::McpToolCallBegin(begin_event) => { let notification = construct_mcp_tool_call_notification( @@ -1415,6 +1475,55 @@ async fn on_exec_approval_response( } } +async fn on_skill_dependencies_request_approval_response( + event_turn_id: String, + receiver: oneshot::Receiver, + conversation: Arc, + request: SkillDependenciesApprovalRequestEvent, +) { + let response = receiver.await; + let decision = match response { + Ok(value) => { + serde_json::from_value::(value) + .unwrap_or_else(|err| { + error!("failed to deserialize SkillDependenciesRequestApprovalResponse: {err}"); + SkillDependenciesRequestApprovalResponse { + decision: SkillDependenciesApprovalDecision::RunAnyway, + } + }) + .decision + } + Err(err) => { + error!("request failed: {err:?}"); + SkillDependenciesApprovalDecision::RunAnyway + } + }; + + let selected_label = match decision { + SkillDependenciesApprovalDecision::Install => request.install.label, + SkillDependenciesApprovalDecision::RunAnyway => request.run_anyway.label, + }; + + let mut answers = HashMap::new(); + answers.insert( + request.question_id, + CoreRequestUserInputAnswer { + answers: vec![selected_label], + }, + ); + let response = CoreRequestUserInputResponse { answers }; + + if let Err(err) = conversation + .submit(Op::UserInputAnswer { + id: event_turn_id, + response, + }) + .await + { + error!("failed to submit UserInputAnswer: {err}"); + } +} + async fn on_request_user_input_response( event_turn_id: String, receiver: oneshot::Receiver, diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 95ec61f20a..d3859004c5 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -9,6 +9,7 @@ mod output_schema; mod rate_limits; mod request_user_input; mod review; +mod skill_dependencies; mod thread_archive; mod thread_fork; mod thread_list; diff --git a/codex-rs/app-server/tests/suite/v2/skill_dependencies.rs b/codex-rs/app-server/tests/suite/v2/skill_dependencies.rs new file mode 100644 index 0000000000..984761d666 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/skill_dependencies.rs @@ -0,0 +1,176 @@ +use anyhow::Result; +use app_test_support::McpProcess; +use app_test_support::create_final_assistant_message_sse_response; +use app_test_support::create_mock_responses_server_sequence; +use app_test_support::to_response; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ServerRequest; +use codex_app_server_protocol::SkillDependenciesApprovalDecision; +use codex_app_server_protocol::SkillDependenciesRequestApprovalResponse; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::UserInput as V2UserInput; +use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::ModeKind; +use codex_protocol::config_types::Settings; +use codex_protocol::openai_models::ReasoningEffort; +use pretty_assertions::assert_eq; +use std::path::Path; +use std::path::PathBuf; +use tokio::time::timeout; + +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn skill_dependencies_request_approval_round_trip() -> Result<()> { + let codex_home = tempfile::TempDir::new()?; + let responses = vec![create_final_assistant_message_sse_response("done")?]; + let server = create_mock_responses_server_sequence(responses).await; + create_config_toml(codex_home.path(), &server.uri())?; + let skill_path = write_skill_files(codex_home.path())?; + let skill_path = std::fs::canonicalize(&skill_path).unwrap_or(skill_path); + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let thread_start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; + + let turn_start_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![ + V2UserInput::Text { + text: "use the skill".to_string(), + text_elements: Vec::new(), + }, + V2UserInput::Skill { + name: "dep-skill".to_string(), + path: skill_path, + }, + ], + model: Some("mock-model".to_string()), + effort: Some(ReasoningEffort::Medium), + collaboration_mode: Some(CollaborationMode { + mode: ModeKind::Plan, + settings: Settings { + model: "mock-model".to_string(), + reasoning_effort: Some(ReasoningEffort::Medium), + developer_instructions: None, + }, + }), + ..Default::default() + }) + .await?; + let turn_start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_start_id)), + ) + .await??; + let TurnStartResponse { turn, .. } = to_response(turn_start_resp)?; + + let server_req = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_request_message(), + ) + .await??; + let ServerRequest::SkillDependenciesRequestApproval { request_id, params } = server_req else { + panic!("expected SkillDependenciesRequestApproval request, got: {server_req:?}"); + }; + + assert_eq!(params.thread_id, thread.id); + assert_eq!(params.turn_id, turn.id); + assert_eq!(params.item_id, "skill-dep-skill-mcp-deps"); + assert_eq!(params.header, "Missing MCP dependency"); + assert_eq!( + params.question, + "The \"dep-skill\" skill depends on MCP server(s) that are not loaded: example-mcp. What would you like to do?" + ); + assert_eq!(params.run_anyway.label, "Run anyway"); + assert_eq!( + params.run_anyway.description, + "Proceed without installing. The skill may not work as expected." + ); + assert_eq!(params.install.label, "Install example-mcp"); + assert_eq!( + params.install.description, + "Install and configure the example-mcp MCP server." + ); + + mcp.send_response( + request_id, + serde_json::to_value(SkillDependenciesRequestApprovalResponse { + decision: SkillDependenciesApprovalDecision::RunAnyway, + })?, + ) + .await?; + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("codex/event/task_complete"), + ) + .await??; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(()) +} + +fn create_config_toml(codex_home: &Path, server_uri: &str) -> std::io::Result<()> { + let config_toml = codex_home.join("config.toml"); + std::fs::write( + config_toml, + format!( + r#" +model = "mock-model" +approval_policy = "untrusted" +sandbox_mode = "read-only" + +model_provider = "mock_provider" + +[features] +collaboration_modes = true + +[model_providers.mock_provider] +name = "Mock provider for test" +base_url = "{server_uri}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 +"# + ), + ) +} + +fn write_skill_files(codex_home: &Path) -> std::io::Result { + let skill_dir = codex_home.join("skills").join("dep-skill"); + std::fs::create_dir_all(&skill_dir)?; + + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: dep-skill\ndescription: Skill dependency test\n---\n\nTest skill.\n", + )?; + + std::fs::write( + skill_dir.join("SKILL.toml"), + "[dependencies]\n[[dependencies.tools]]\ntype = \"mcp\"\nvalue = \"example-mcp\"\ndescription = \"Example MCP\"\ntransport = \"streamable_http\"\nurl = \"https://example.com\"\n", + )?; + + Ok(skill_dir.join("SKILL.md")) +} diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 9fc2bae165..e136201d52 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -132,6 +132,7 @@ use crate::protocol::RequestUserInputEvent; use crate::protocol::ReviewDecision; use crate::protocol::SandboxPolicy; use crate::protocol::SessionConfiguredEvent; +use crate::protocol::SkillDependenciesApprovalRequestEvent; use crate::protocol::SkillErrorInfo; use crate::protocol::SkillInterface as ProtocolSkillInterface; use crate::protocol::SkillMetadata as ProtocolSkillMetadata; @@ -1106,7 +1107,7 @@ impl Session { .await } - async fn get_config(&self) -> std::sync::Arc { + pub(crate) async fn get_config(&self) -> std::sync::Arc { let state = self.state.lock().await; state .session_configuration @@ -1470,6 +1471,34 @@ impl Session { rx_response.await.ok() } + pub async fn request_skill_dependencies_approval( + &self, + turn_context: &TurnContext, + mut event: SkillDependenciesApprovalRequestEvent, + ) -> Option { + let sub_id = turn_context.sub_id.clone(); + let (tx_response, rx_response) = oneshot::channel(); + let event_id = sub_id.clone(); + let prev_entry = { + let mut active = self.active_turn.lock().await; + match active.as_mut() { + Some(at) => { + let mut ts = at.turn_state.lock().await; + ts.insert_pending_user_input(sub_id, tx_response) + } + None => None, + } + }; + if prev_entry.is_some() { + warn!("Overwriting existing pending user input for sub_id: {event_id}"); + } + + event.turn_id = event_id.clone(); + let event = EventMsg::SkillDependenciesApprovalRequest(event); + self.send_event(turn_context, event).await; + rx_response.await.ok() + } + pub async fn notify_user_input_response( &self, sub_id: &str, @@ -2931,7 +2960,14 @@ pub(crate) async fn run_turn( let SkillInjections { items: skill_items, warnings: skill_warnings, - } = build_skill_injections(&input, skills_outcome.as_ref(), Some(&otel_manager)).await; + } = build_skill_injections( + sess.as_ref(), + turn_context.as_ref(), + &input, + skills_outcome.as_ref(), + Some(&otel_manager), + ) + .await; for message in skill_warnings { sess.send_event(&turn_context, EventMsg::Warning(WarningEvent { message })) diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 9a94f988d5..b8071ffa68 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -12,6 +12,7 @@ use codex_protocol::protocol::ExecApprovalRequestEvent; use codex_protocol::protocol::Op; use codex_protocol::protocol::RequestUserInputEvent; use codex_protocol::protocol::SessionSource; +use codex_protocol::protocol::SkillDependenciesApprovalRequestEvent; use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::Submission; use codex_protocol::request_user_input::RequestUserInputArgs; @@ -250,6 +251,20 @@ async fn forward_events( ) .await; } + Event { + id, + msg: EventMsg::SkillDependenciesApprovalRequest(event), + } => { + handle_skill_dependencies_approval( + &codex, + id, + &parent_session, + &parent_ctx, + event, + &cancel_token, + ) + .await; + } other => { match tx_sub.send(other).or_cancel(&cancel_token).await { Ok(Ok(())) => {} @@ -378,6 +393,25 @@ async fn handle_request_user_input( let _ = codex.submit(Op::UserInputAnswer { id, response }).await; } +async fn handle_skill_dependencies_approval( + codex: &Codex, + id: String, + parent_session: &Session, + parent_ctx: &TurnContext, + event: SkillDependenciesApprovalRequestEvent, + cancel_token: &CancellationToken, +) { + let response_fut = parent_session.request_skill_dependencies_approval(parent_ctx, event); + let response = await_user_input_with_cancel( + response_fut, + parent_session, + &parent_ctx.sub_id, + cancel_token, + ) + .await; + let _ = codex.submit(Op::UserInputAnswer { id, response }).await; +} + async fn await_user_input_with_cancel( fut: F, parent_session: &Session, diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 434db3b2fa..f2f29d1b13 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -312,6 +312,36 @@ pub(crate) struct McpConnectionManager { } impl McpConnectionManager { + pub(crate) fn startup_futures_for( + &self, + server_names: &[String], + ) -> Vec)>> { + server_names + .iter() + .map(|server_name| { + if let Some(client) = self.clients.get(server_name) { + let server_name = server_name.clone(); + let client = client.clone(); + async move { + let outcome = client.client().await; + let result = match outcome { + Ok(_) => Ok(()), + Err(StartupOutcomeError::Cancelled) => { + Err("MCP startup cancelled".to_string()) + } + Err(StartupOutcomeError::Failed { error }) => Err(error), + }; + (server_name, result) + } + .boxed() + } else { + let server_name = server_name.clone(); + async move { (server_name, Err("unknown MCP server".to_string())) }.boxed() + } + }) + .collect() + } + pub async fn initialize( &mut self, mcp_servers: &HashMap, @@ -416,6 +446,125 @@ impl McpConnectionManager { }); } + pub async fn add_servers( + &mut self, + mcp_servers: &HashMap, + store_mode: OAuthCredentialsStoreMode, + auth_entries: HashMap, + tx_event: Sender, + cancel_token: CancellationToken, + sandbox_state: SandboxState, + ) { + if cancel_token.is_cancelled() { + return; + } + + let mut join_set = JoinSet::new(); + let mut started_any = false; + let elicitation_requests = self.elicitation_requests.clone(); + + for (server_name, cfg) in mcp_servers.iter().filter(|(_, cfg)| cfg.enabled) { + if self.clients.contains_key(server_name) { + continue; + } + + let server_name = server_name.clone(); + let cancel_token = cancel_token.child_token(); + let _ = emit_update( + &tx_event, + McpStartupUpdateEvent { + server: server_name.clone(), + status: McpStartupStatus::Starting, + }, + ) + .await; + + let async_managed_client = AsyncManagedClient::new( + server_name.clone(), + cfg.clone(), + store_mode, + cancel_token.clone(), + tx_event.clone(), + elicitation_requests.clone(), + ); + self.clients + .insert(server_name.clone(), async_managed_client.clone()); + let tx_event = tx_event.clone(); + let auth_entry = auth_entries.get(&server_name).cloned(); + let sandbox_state = sandbox_state.clone(); + join_set.spawn(async move { + let outcome = async_managed_client.client().await; + if cancel_token.is_cancelled() { + return (server_name, Err(StartupOutcomeError::Cancelled)); + } + let status = match &outcome { + Ok(_) => { + if let Err(err) = async_managed_client + .notify_sandbox_state_change(&sandbox_state) + .await + { + warn!( + "Failed to notify sandbox state to MCP server {server_name}: {err:#}", + ); + } + McpStartupStatus::Ready + } + Err(error) => { + let error_str = mcp_init_error_display( + server_name.as_str(), + auth_entry.as_ref(), + error, + ); + McpStartupStatus::Failed { error: error_str } + } + }; + + let _ = emit_update( + &tx_event, + McpStartupUpdateEvent { + server: server_name.clone(), + status, + }, + ) + .await; + + (server_name, outcome) + }); + started_any = true; + } + + if !started_any { + return; + } + + tokio::spawn(async move { + let outcomes = join_set.join_all().await; + let mut summary = McpStartupCompleteEvent::default(); + for (server_name, outcome) in outcomes { + match outcome { + Ok(_) => summary.ready.push(server_name), + Err(StartupOutcomeError::Cancelled) => summary.cancelled.push(server_name), + Err(StartupOutcomeError::Failed { error }) => { + summary.failed.push(McpStartupFailure { + server: server_name, + error, + }) + } + } + } + let _ = tx_event + .send(Event { + id: INITIAL_SUBMIT_ID.to_owned(), + msg: EventMsg::McpStartupComplete(summary), + }) + .await; + }); + } + + pub(crate) fn has_server(&self, server_name: &str) -> bool { + self.clients.contains_key(server_name) + } + async fn client_by_name(&self, name: &str) -> Result { self.clients .get(name) diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 2d4a79cc22..7070bfa6f4 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -69,6 +69,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::ExecApprovalRequest(_) | EventMsg::RequestUserInput(_) | EventMsg::ElicitationRequest(_) + | EventMsg::SkillDependenciesApprovalRequest(_) | EventMsg::ApplyPatchApprovalRequest(_) | EventMsg::BackgroundEvent(_) | EventMsg::StreamError(_) @@ -85,6 +86,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::PlanUpdate(_) | EventMsg::ShutdownComplete | EventMsg::ViewImageToolCall(_) + | EventMsg::SkillRead(_) | EventMsg::DeprecationNotice(_) | EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) diff --git a/codex-rs/core/src/skills/dependencies.rs b/codex-rs/core/src/skills/dependencies.rs new file mode 100644 index 0000000000..890a8e55ba --- /dev/null +++ b/codex-rs/core/src/skills/dependencies.rs @@ -0,0 +1,661 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::env; +use std::time::Duration; + +use codex_protocol::approvals::SkillDependenciesApprovalOption; +use codex_protocol::approvals::SkillDependenciesApprovalRequestEvent; +use codex_protocol::protocol::McpAuthStatus; +use codex_rmcp_client::perform_oauth_login; +use codex_rmcp_client::supports_oauth_login; +use futures::future::join_all; +use shlex::split as shlex_split; +use tokio::time::timeout; +use tracing::warn; + +use crate::codex::Session; +use crate::codex::TurnContext; +use crate::config::Config; +use crate::config::edit::ConfigEditsBuilder; +use crate::config::find_codex_home; +use crate::config::load_global_mcp_servers; +use crate::config::types::McpServerConfig; +use crate::config::types::McpServerTransportConfig; +use crate::mcp::auth::McpAuthStatusEntry; +use crate::mcp::auth::compute_auth_statuses; +use crate::mcp::effective_mcp_servers; +use crate::mcp_connection_manager::SandboxState; +use crate::skills::SkillMetadata; +use crate::skills::model::SkillDependencies; +use crate::skills::model::SkillToolDependency; + +pub(crate) async fn handle_skill_dependencies( + session: &Session, + turn: &TurnContext, + call_id: String, + skill: &SkillMetadata, +) -> Option { + let Some(dependencies) = skill.dependencies.as_ref() else { + return None; + }; + + let missing_mcps = collect_missing_mcp_dependencies(session, dependencies).await; + if !missing_mcps.is_empty() { + let install = prompt_missing_mcp_dependencies(session, turn, call_id, skill, &missing_mcps) + .await + .unwrap_or(false); + if install { + install_missing_mcp_dependencies(session, turn, skill, &missing_mcps).await; + } + } + + None +} + +async fn collect_missing_mcp_dependencies( + session: &Session, + dependencies: &SkillDependencies, +) -> Vec { + let mut seen = HashSet::new(); + let mcp_dependency_names = collect_mcp_dependency_names(&dependencies.tools); + let auth_entries = collect_mcp_auth_entries(session, &mcp_dependency_names).await; + let mcp_connection_manager = session.services.mcp_connection_manager.read().await; + dependencies + .tools + .iter() + .filter(|dependency| dependency.tool_type.eq_ignore_ascii_case("mcp")) + .filter_map(|dependency| { + let name = dependency.value.trim(); + if name.is_empty() || !seen.insert(name.to_string()) { + return None; + } + if mcp_connection_manager.has_server(name) { + if auth_entries.get(name).is_some_and(mcp_auth_missing) { + Some(dependency.clone()) + } else { + None + } + } else { + Some(dependency.clone()) + } + }) + .collect() +} + +fn collect_mcp_dependency_names(dependencies: &[SkillToolDependency]) -> HashSet { + dependencies + .iter() + .filter_map(|dependency| { + if !dependency.tool_type.eq_ignore_ascii_case("mcp") { + return None; + } + let name = dependency.value.trim(); + if name.is_empty() { + None + } else { + Some(name.to_string()) + } + }) + .collect() +} + +async fn collect_mcp_auth_entries( + session: &Session, + mcp_dependency_names: &HashSet, +) -> HashMap { + if mcp_dependency_names.is_empty() { + return HashMap::new(); + } + + let config = session.get_config().await; + let auth = session.services.auth_manager.auth().await; + let mcp_servers = effective_mcp_servers(&config, auth.as_ref()) + .into_iter() + .filter(|(name, _)| mcp_dependency_names.contains(name)) + .collect::>(); + if mcp_servers.is_empty() { + return HashMap::new(); + } + + compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode).await +} + +fn mcp_auth_missing(entry: &McpAuthStatusEntry) -> bool { + entry.auth_status == McpAuthStatus::NotLoggedIn || bearer_token_env_var_missing(&entry.config) +} + +fn mcp_needs_oauth_login(entry: &McpAuthStatusEntry) -> bool { + entry.auth_status == McpAuthStatus::NotLoggedIn +} + +fn bearer_token_env_var_missing(config: &McpServerConfig) -> bool { + let McpServerTransportConfig::StreamableHttp { + bearer_token_env_var: Some(env_var), + .. + } = &config.transport + else { + return false; + }; + + match env::var(env_var) { + Ok(value) => value.is_empty(), + Err(env::VarError::NotPresent) | Err(env::VarError::NotUnicode(_)) => true, + } +} + +fn bearer_token_env_var_name(config: &McpServerConfig) -> Option<&str> { + match &config.transport { + McpServerTransportConfig::StreamableHttp { + bearer_token_env_var: Some(env_var), + .. + } => Some(env_var.as_str()), + _ => None, + } +} + +async fn prompt_missing_mcp_dependencies( + session: &Session, + turn: &TurnContext, + call_id: String, + skill: &SkillMetadata, + missing_mcps: &[SkillToolDependency], +) -> Option { + let missing_names = missing_mcps + .iter() + .map(|dependency| dependency.value.trim().to_string()) + .collect::>(); + if missing_names.is_empty() { + return None; + } + + let skill_name = skill.name.as_str(); + let question_id = "missing_mcp_dependencies".to_string(); + let missing_list = missing_names.join(", "); + let run_anyway_label = "Run anyway".to_string(); + let install_label = if missing_names.len() == 1 { + let install_target = &missing_names[0]; + format!("Install {install_target}") + } else { + "Install missing MCPs".to_string() + }; + let header = if missing_names.len() == 1 { + "Missing MCP dependency".to_string() + } else { + "Missing MCP dependencies".to_string() + }; + let prompt = format!( + "The \"{skill_name}\" skill depends on MCP server(s) that are not loaded or missing required authentication: {missing_list}. What would you like to do?" + ); + let description = if missing_names.len() == 1 { + let install_target = &missing_names[0]; + format!("Install and configure the {install_target} MCP server.") + } else { + "Install and configure the missing MCP servers.".to_string() + }; + let event = SkillDependenciesApprovalRequestEvent { + call_id, + turn_id: turn.sub_id.clone(), + question_id: question_id.clone(), + header, + question: prompt, + run_anyway: SkillDependenciesApprovalOption { + label: run_anyway_label.clone(), + description: "Proceed without installing. The skill may not work as expected." + .to_string(), + }, + install: SkillDependenciesApprovalOption { + label: install_label.clone(), + description, + }, + }; + + let response = session + .request_skill_dependencies_approval(turn, event) + .await; + + let selected = response + .as_ref() + .and_then(|response| response.answers.get(&question_id)) + .and_then(|answer| answer.answers.first()); + Some(matches!(selected, Some(answer) if *answer == install_label)) +} + +async fn install_missing_mcp_dependencies( + session: &Session, + turn: &TurnContext, + skill: &SkillMetadata, + missing_mcps: &[SkillToolDependency], +) { + if missing_mcps.is_empty() { + return; + } + + let pending = missing_mcps + .iter() + .filter(|dependency| !dependency.value.trim().is_empty()) + .collect::>(); + if pending.is_empty() { + return; + } + + let total = pending.len(); + let mcp_dependency_names = collect_mcp_dependency_names(missing_mcps); + let auth_entries = collect_mcp_auth_entries(session, &mcp_dependency_names).await; + let skill_name = skill.name.as_str(); + let plural = if total == 1 { + "dependency" + } else { + "dependencies" + }; + session + .notify_background_event( + turn, + format!("Installing {total} MCP {plural} for \"{skill_name}\"."), + ) + .await; + let config = match Config::load_with_cli_overrides(Vec::new()).await { + Ok(config) => config, + Err(err) => { + warn!("Failed to load config for MCP installs (skill={skill_name}): {err}"); + session + .notify_background_event( + turn, + format!("Failed to load config for MCP installs (skill={skill_name}): {err}"), + ) + .await; + return; + } + }; + + let codex_home = match find_codex_home() { + Ok(codex_home) => codex_home, + Err(err) => { + warn!("Failed to resolve CODEX_HOME for MCP installs (skill={skill_name}): {err}"); + session + .notify_background_event( + turn, + format!( + "Failed to resolve CODEX_HOME for MCP installs (skill={skill_name}): {err}" + ), + ) + .await; + return; + } + }; + + let mut servers = match load_global_mcp_servers(&codex_home).await { + Ok(servers) => servers, + Err(err) => { + warn!("Failed to load MCP servers for MCP installs (skill={skill_name}): {err}"); + session + .notify_background_event( + turn, + format!( + "Failed to load MCP servers for MCP installs (skill={skill_name}): {err}" + ), + ) + .await; + return; + } + }; + + const OAUTH_LOGIN_TIMEOUT_SECS: u64 = 300; + let oauth_login_timeout = Duration::from_secs(OAUTH_LOGIN_TIMEOUT_SECS); + + for (index, dependency) in pending.into_iter().enumerate() { + let name = dependency.value.trim(); + let step = index + 1; + let progress_prefix = format!("MCP dependency {step}/{total}: {name}"); + session + .notify_background_event(turn, format!("{progress_prefix} - starting")) + .await; + if !is_valid_mcp_server_name(name) { + warn!("Invalid MCP server name '{name}' for skill '{skill_name}'."); + session + .notify_background_event( + turn, + format!("{progress_prefix} - skipped (invalid name)"), + ) + .await; + continue; + } + + let auth_entry = auth_entries.get(name); + let auth_missing = auth_entry.is_some_and(mcp_auth_missing); + let needs_oauth_login = + auth_entry.is_none() || auth_entry.is_some_and(mcp_needs_oauth_login); + + let mut installed_servers: HashMap = HashMap::new(); + let mut oauth_transport: Option = None; + let mut config_changed = false; + let mut did_oauth_attempt = false; + + if let Some(existing) = servers.get(name).cloned() { + if existing.enabled { + if needs_oauth_login { + oauth_transport = Some(existing.transport.clone()); + } + } else { + let mut updated = existing.clone(); + updated.enabled = true; + updated.disabled_reason = None; + servers.insert(name.to_string(), updated.clone()); + installed_servers.insert(name.to_string(), updated.clone()); + oauth_transport = Some(updated.transport.clone()); + config_changed = true; + } + } else { + let transport = match resolve_mcp_transport(dependency) { + Ok(transport) => transport, + Err(err) => { + warn!( + "Failed to resolve MCP transport for '{name}' (skill={skill_name}): {err}" + ); + session + .notify_background_event( + turn, + format!("{progress_prefix} - failed to resolve transport: {err}"), + ) + .await; + continue; + } + }; + let new_entry = McpServerConfig { + transport: transport.clone(), + enabled: true, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + }; + + servers.insert(name.to_string(), new_entry.clone()); + installed_servers.insert(name.to_string(), new_entry.clone()); + oauth_transport = Some(transport); + config_changed = true; + } + + if config_changed + && let Err(err) = ConfigEditsBuilder::new(&codex_home) + .replace_mcp_servers(&servers) + .apply() + .await + { + warn!("Failed to write MCP servers for skill '{skill_name}': {err}"); + session + .notify_background_event( + turn, + format!("{progress_prefix} - failed to update config: {err}"), + ) + .await; + continue; + } + + if config_changed { + session + .notify_background_event(turn, format!("{progress_prefix} - config updated")) + .await; + } + + if needs_oauth_login + && let Some(McpServerTransportConfig::StreamableHttp { + url, + bearer_token_env_var: None, + http_headers, + env_http_headers, + }) = oauth_transport + { + did_oauth_attempt = true; + session + .notify_background_event(turn, format!("{progress_prefix} - starting OAuth login")) + .await; + match supports_oauth_login(&url).await { + Ok(true) => { + match timeout( + oauth_login_timeout, + perform_oauth_login( + name, + &url, + config.mcp_oauth_credentials_store_mode, + http_headers.clone(), + env_http_headers.clone(), + &[], + config.mcp_oauth_callback_port, + ), + ) + .await + { + Ok(Ok(())) => { + session + .notify_background_event( + turn, + format!("{progress_prefix} - OAuth login complete"), + ) + .await; + } + Ok(Err(err)) => { + warn!( + "OAuth login failed for MCP server '{name}' (skill={skill_name}): {err}" + ); + session + .notify_background_event( + turn, + format!("{progress_prefix} - OAuth login failed: {err}"), + ) + .await; + } + Err(_) => { + warn!( + "OAuth login timed out for MCP server '{name}' (skill={skill_name}) after {OAUTH_LOGIN_TIMEOUT_SECS} seconds" + ); + session + .notify_background_event( + turn, + format!( + "{progress_prefix} - OAuth login timed out after {OAUTH_LOGIN_TIMEOUT_SECS} seconds" + ), + ) + .await; + } + } + } + Ok(false) => { + session + .notify_background_event( + turn, + format!("{progress_prefix} - OAuth not supported; skipping login"), + ) + .await; + } + Err(err) => { + warn!( + "OAuth support check failed for MCP server '{name}' (skill={skill_name}): {err}" + ); + session + .notify_background_event( + turn, + format!("{progress_prefix} - OAuth check failed: {err}"), + ) + .await; + } + } + } + + if auth_missing && !needs_oauth_login && installed_servers.is_empty() { + let env_var = auth_entry.and_then(|entry| bearer_token_env_var_name(&entry.config)); + let message = env_var.map_or_else( + || format!("{progress_prefix} - authentication required; set bearer token env var"), + |env_var| format!("{progress_prefix} - missing {env_var}; set it and retry"), + ); + session.notify_background_event(turn, message).await; + continue; + } + + if installed_servers.is_empty() { + let message = if did_oauth_attempt { + format!("{progress_prefix} - authentication attempt complete") + } else { + format!("{progress_prefix} - no install steps required") + }; + session.notify_background_event(turn, message).await; + continue; + } + + let outcomes = + hot_reload_installed_mcp_servers(session, turn, installed_servers, &config).await; + if let Some((_, result)) = outcomes + .into_iter() + .find(|(server_name, _)| server_name == name) + { + match result { + Ok(()) => { + session + .notify_background_event( + turn, + format!("{progress_prefix} - startup complete"), + ) + .await; + } + Err(error) => { + session + .notify_background_event( + turn, + format!("{progress_prefix} - startup failed: {error}"), + ) + .await; + } + } + } + } +} + +async fn hot_reload_installed_mcp_servers( + session: &Session, + turn: &TurnContext, + installed_servers: HashMap, + config: &Config, +) -> Vec<(String, Result<(), String>)> { + let server_names = installed_servers.keys().cloned().collect::>(); + let auth_statuses = compute_auth_statuses( + installed_servers.iter(), + config.mcp_oauth_credentials_store_mode, + ) + .await; + let sandbox_state = SandboxState { + sandbox_policy: turn.sandbox_policy.clone(), + codex_linux_sandbox_exe: turn.codex_linux_sandbox_exe.clone(), + sandbox_cwd: turn.cwd.clone(), + }; + let cancel_token = session + .services + .mcp_startup_cancellation_token + .lock() + .await + .clone(); + let tx_event = session.get_tx_event(); + session + .services + .mcp_connection_manager + .write() + .await + .add_servers( + &installed_servers, + config.mcp_oauth_credentials_store_mode, + auth_statuses, + tx_event, + cancel_token, + sandbox_state, + ) + .await; + + if server_names.is_empty() { + return Vec::new(); + } + + let startup_futures = session + .services + .mcp_connection_manager + .read() + .await + .startup_futures_for(&server_names); + let outcomes = join_all(startup_futures).await; + for (server_name, result) in &outcomes { + if let Err(error) = result { + warn!("MCP server '{server_name}' failed to start: {error}"); + } + } + outcomes +} + +fn resolve_mcp_transport( + dependency: &SkillToolDependency, +) -> Result { + let transport = dependency.transport.as_deref(); + let url = dependency.url.as_deref().map(str::trim); + let value = dependency.value.trim(); + + if transport + .map(|transport| transport.eq_ignore_ascii_case("streamable_http")) + .unwrap_or(false) + { + let url = url + .filter(|url| !url.is_empty()) + .ok_or_else(|| "missing URL for streamable_http transport".to_string())?; + return Ok(McpServerTransportConfig::StreamableHttp { + url: url.to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }); + } + + if transport + .map(|transport| transport.eq_ignore_ascii_case("stdio")) + .unwrap_or(false) + { + let command_line = url + .filter(|command_line| !command_line.is_empty()) + .ok_or_else(|| "missing command for stdio transport".to_string())?; + let tokens = shlex_split(command_line).unwrap_or_else(|| { + command_line + .split_whitespace() + .map(ToString::to_string) + .collect() + }); + let mut iter = tokens.into_iter(); + let command = iter + .next() + .ok_or_else(|| "missing command for stdio transport".to_string())?; + let args = iter.collect::>(); + return Ok(McpServerTransportConfig::Stdio { + command, + args, + env: None, + env_vars: Vec::new(), + cwd: None, + }); + } + + if transport.is_none() + && let Some(url) = url.filter(|url| !url.is_empty()) + { + return Ok(McpServerTransportConfig::StreamableHttp { + url: url.to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }); + } + + Err(format!( + "unsupported MCP dependency (name={value}, transport={transport:?}, url={url:?})" + )) +} + +fn is_valid_mcp_server_name(name: &str) -> bool { + !name.is_empty() + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') +} diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index 9aa12d775c..bc71a5ed30 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -1,10 +1,14 @@ use std::collections::HashSet; use std::path::PathBuf; +use crate::codex::Session; +use crate::codex::TurnContext; use crate::instructions::SkillInstructions; use crate::skills::SkillLoadOutcome; use crate::skills::SkillMetadata; +use crate::skills::handle_skill_dependencies; use codex_otel::OtelManager; +use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; use tokio::fs; @@ -16,6 +20,8 @@ pub(crate) struct SkillInjections { } pub(crate) async fn build_skill_injections( + session: &Session, + turn: &TurnContext, inputs: &[UserInput], skills: Option<&SkillLoadOutcome>, otel: Option<&OtelManager>, @@ -40,6 +46,18 @@ pub(crate) async fn build_skill_injections( }; for skill in mentioned_skills { + let call_id = { + let skill_name = skill.name.as_str(); + format!("skill-{skill_name}-mcp-deps") + }; + if let Some(message) = handle_skill_dependencies(session, turn, call_id, &skill).await { + result.items.push(ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { text: message }], + end_turn: None, + }); + } match fs::read_to_string(&skill.path).await { Ok(contents) => { emit_skill_injected_metric(otel, &skill, "ok"); @@ -51,11 +69,9 @@ pub(crate) async fn build_skill_injections( } Err(err) => { emit_skill_injected_metric(otel, &skill, "error"); - let message = format!( - "Failed to load skill {name} at {path}: {err:#}", - name = skill.name, - path = skill.path.display() - ); + let skill_name = skill.name.as_str(); + let skill_path = skill.path.display(); + let message = format!("Failed to load skill {skill_name} at {skill_path}: {err:#}"); result.warnings.push(message); } } @@ -92,7 +108,98 @@ fn collect_explicit_skill_mentions( { selected.push(skill.clone()); } + + if let UserInput::Text { text, .. } = input { + for name in extract_inline_skill_mentions(text) { + if let Some(skill) = skills.iter().find(|skill| skill.name == name) + && !disabled_paths.contains(&skill.path) + && seen.insert(name.clone()) + { + selected.push(skill.clone()); + } + } + } } selected } + +fn extract_inline_skill_mentions(text: &str) -> Vec { + const MAX_SKILL_NAME_LEN: usize = 99; + let mut names = Vec::new(); + let bytes = text.as_bytes(); + let mut index = 0; + + while index < bytes.len() { + if bytes[index] == b'$' { + let prev_is_word = index > 0 && is_word_boundary_char(bytes[index - 1]); + if !prev_is_word { + let name_start = index + 1; + let mut name_end = name_start; + while name_end < bytes.len() && is_skill_name_char(bytes[name_end]) { + name_end += 1; + } + let name_len = name_end.saturating_sub(name_start); + if name_len > 0 && name_len <= MAX_SKILL_NAME_LEN { + let name = &text[name_start..name_end]; + names.push(name.to_string()); + } + index = name_end; + continue; + } + } + index += 1; + } + + names +} + +fn is_word_boundary_char(byte: u8) -> bool { + byte.is_ascii_alphanumeric() || byte == b'_' +} + +fn is_skill_name_char(byte: u8) -> bool { + byte.is_ascii_alphanumeric() || byte == b'_' || byte == b'-' +} + +#[cfg(test)] +mod tests { + use super::extract_inline_skill_mentions; + use pretty_assertions::assert_eq; + + #[test] + fn extracts_inline_skill_mentions() { + let text = "Use $notion-research-documentation to generate docs."; + let names = extract_inline_skill_mentions(text); + assert_eq!(names, vec!["notion-research-documentation".to_string()]); + } + + #[test] + fn extracts_mentions_inside_markdown_links() { + let text = "This is [$not-a-link] and [$ok](path) for later."; + let names = extract_inline_skill_mentions(text); + assert_eq!(names, vec!["not-a-link".to_string(), "ok".to_string()]); + } + + #[test] + fn extracts_dash_names() { + let text = "Use $spaced-name please."; + let names = extract_inline_skill_mentions(text); + assert_eq!(names, vec!["spaced-name".to_string()]); + } + + #[test] + fn ignores_names_in_the_middle_of_words() { + let text = "ignore foo$bar but accept $good_name."; + let names = extract_inline_skill_mentions(text); + assert_eq!(names, vec!["good_name".to_string()]); + } + + #[test] + fn skips_overly_long_names() { + let long_name = "a".repeat(100); + let text = format!("Use ${long_name} for now."); + let names = extract_inline_skill_mentions(&text); + assert_eq!(names, Vec::::new()); + } +} diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 09c988a15b..f4ba1041d9 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -1,10 +1,12 @@ use crate::config::Config; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; +use crate::skills::model::SkillDependencies; use crate::skills::model::SkillError; use crate::skills::model::SkillInterface; use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; +use crate::skills::model::SkillToolDependency; use crate::skills::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::protocol::SkillScope; @@ -38,6 +40,8 @@ struct SkillFrontmatterMetadata { struct SkillToml { #[serde(default)] interface: Option, + #[serde(default)] + dependencies: Option, } #[derive(Debug, Default, Deserialize)] @@ -50,6 +54,22 @@ struct Interface { default_prompt: Option, } +#[derive(Debug, Default, Deserialize)] +struct Dependencies { + #[serde(default)] + tools: Vec, +} + +#[derive(Debug, Default, Deserialize)] +struct ToolDependency { + #[serde(rename = "type")] + tool_type: Option, + value: Option, + description: Option, + transport: Option, + url: Option, +} + const SKILLS_FILENAME: &str = "SKILL.md"; const SKILLS_TOML_FILENAME: &str = "SKILL.toml"; const SKILLS_DIR_NAME: &str = "skills"; @@ -345,7 +365,10 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result Result Option { +#[derive(Debug, Default)] +struct SkillTomlMetadata { + interface: Option, + dependencies: Option, +} + +fn load_skill_toml(skill_path: &Path) -> SkillTomlMetadata { // Fail open: optional SKILL.toml metadata should not block loading SKILL.md. - let skill_dir = skill_path.parent()?; + let Some(skill_dir) = skill_path.parent() else { + return SkillTomlMetadata::default(); + }; let interface_path = skill_dir.join(SKILLS_TOML_FILENAME); if !interface_path.exists() { - return None; + return SkillTomlMetadata::default(); } let contents = match fs::read_to_string(&interface_path) { @@ -384,7 +416,7 @@ fn load_skill_interface(skill_path: &Path) -> Option { "ignoring {path}: failed to read SKILL.toml: {error}", path = interface_path.display() ); - return None; + return SkillTomlMetadata::default(); } }; let parsed: SkillToml = match toml::from_str(&contents) { @@ -394,38 +426,96 @@ fn load_skill_interface(skill_path: &Path) -> Option { "ignoring {path}: invalid TOML: {error}", path = interface_path.display() ); - return None; + return SkillTomlMetadata::default(); } }; - let interface = parsed.interface?; - let interface = SkillInterface { - display_name: resolve_str( - interface.display_name, - MAX_NAME_LEN, - "interface.display_name", - ), - short_description: resolve_str( - interface.short_description, - MAX_SHORT_DESCRIPTION_LEN, - "interface.short_description", - ), - icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small), - icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large), - brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"), - default_prompt: resolve_str( - interface.default_prompt, - MAX_DEFAULT_PROMPT_LEN, - "interface.default_prompt", - ), - }; - let has_fields = interface.display_name.is_some() - || interface.short_description.is_some() - || interface.icon_small.is_some() - || interface.icon_large.is_some() - || interface.brand_color.is_some() - || interface.default_prompt.is_some(); - if has_fields { Some(interface) } else { None } + let interface = parsed.interface.and_then(|interface| { + let interface = SkillInterface { + display_name: resolve_str( + interface.display_name, + MAX_NAME_LEN, + "interface.display_name", + ), + short_description: resolve_str( + interface.short_description, + MAX_SHORT_DESCRIPTION_LEN, + "interface.short_description", + ), + icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small), + icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large), + brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"), + default_prompt: resolve_str( + interface.default_prompt, + MAX_DEFAULT_PROMPT_LEN, + "interface.default_prompt", + ), + }; + let has_fields = interface.display_name.is_some() + || interface.short_description.is_some() + || interface.icon_small.is_some() + || interface.icon_large.is_some() + || interface.brand_color.is_some() + || interface.default_prompt.is_some(); + if has_fields { Some(interface) } else { None } + }); + + let dependencies = resolve_dependencies(parsed.dependencies); + + SkillTomlMetadata { + interface, + dependencies, + } +} + +fn resolve_dependencies(dependencies: Option) -> Option { + let dependencies = dependencies?; + let tools = dependencies + .tools + .into_iter() + .filter_map(resolve_tool_dependency) + .collect::>(); + if tools.is_empty() { + None + } else { + Some(SkillDependencies { tools }) + } +} + +fn resolve_tool_dependency(dependency: ToolDependency) -> Option { + let tool_type = resolve_required_str( + dependency.tool_type, + MAX_NAME_LEN, + "dependencies.tools.type", + )?; + let value = resolve_required_str( + dependency.value, + MAX_DESCRIPTION_LEN, + "dependencies.tools.value", + )?; + let description = resolve_str( + dependency.description, + MAX_DESCRIPTION_LEN, + "dependencies.tools.description", + ); + let transport = resolve_str( + dependency.transport, + MAX_NAME_LEN, + "dependencies.tools.transport", + ); + let url = resolve_str( + dependency.url, + MAX_DESCRIPTION_LEN, + "dependencies.tools.url", + ); + + Some(SkillToolDependency { + tool_type, + value, + description, + transport, + url, + }) } fn resolve_asset_path( @@ -511,6 +601,18 @@ fn resolve_str(value: Option, max_len: usize, field: &'static str) -> Op Some(value) } +fn resolve_required_str( + value: Option, + max_len: usize, + field: &'static str, +) -> Option { + let Some(value) = value else { + tracing::warn!("ignoring {field}: value is missing"); + return None; + }; + resolve_str(Some(value), max_len, field) +} + fn resolve_color_str(value: Option, field: &'static str) -> Option { let value = value?; let value = value.trim(); @@ -795,6 +897,7 @@ default_prompt = " default prompt " name: "ui-skill".to_string(), description: "from toml".to_string(), short_description: None, + dependencies: None, interface: Some(SkillInterface { display_name: Some("UI Skill".to_string()), short_description: Some("short desc".to_string()), @@ -809,6 +912,80 @@ default_prompt = " default prompt " ); } + #[tokio::test] + async fn loads_skill_tool_dependencies_from_toml() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "deps-skill", "deps from toml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_interface_at( + skill_dir, + r#" +[[dependencies.tools]] +type = "env_var" +value = "GITHUB_TOKEN" +description = "GitHub API token with repo scopes" + +[[dependencies.tools]] +type = "mcp" +value = "github" +description = "GitHub MCP server" +transport = "streamable_http" +url = "https://example.com/mcp" + +[[dependencies.tools]] +type = "cli" +value = "gh" +description = "GitHub CLI" +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "deps-skill".to_string(), + description: "deps from toml".to_string(), + short_description: None, + dependencies: Some(SkillDependencies { + tools: vec![ + SkillToolDependency { + tool_type: "env_var".to_string(), + value: "GITHUB_TOKEN".to_string(), + description: Some("GitHub API token with repo scopes".to_string()), + transport: None, + url: None, + }, + SkillToolDependency { + tool_type: "mcp".to_string(), + value: "github".to_string(), + description: Some("GitHub MCP server".to_string()), + transport: Some("streamable_http".to_string()), + url: Some("https://example.com/mcp".to_string()), + }, + SkillToolDependency { + tool_type: "cli".to_string(), + value: "gh".to_string(), + description: Some("GitHub CLI".to_string()), + transport: None, + url: None, + }, + ], + }), + interface: None, + path: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + #[tokio::test] async fn accepts_icon_paths_under_assets_dir() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -840,6 +1017,7 @@ icon_large = "./assets/logo.svg" name: "ui-skill".to_string(), description: "from toml".to_string(), short_description: None, + dependencies: None, interface: Some(SkillInterface { display_name: Some("UI Skill".to_string()), short_description: None, @@ -882,6 +1060,7 @@ brand_color = "blue" name: "ui-skill".to_string(), description: "from toml".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::User, @@ -923,6 +1102,7 @@ default_prompt = "{too_long}" name: "ui-skill".to_string(), description: "from toml".to_string(), short_description: None, + dependencies: None, interface: Some(SkillInterface { display_name: Some("UI Skill".to_string()), short_description: None, @@ -966,6 +1146,7 @@ icon_large = "./assets/../logo.svg" name: "ui-skill".to_string(), description: "from toml".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::User, @@ -1008,6 +1189,7 @@ icon_large = "./assets/../logo.svg" name: "linked-skill".to_string(), description: "from link".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&shared_skill_path), scope: SkillScope::User, @@ -1066,6 +1248,7 @@ icon_large = "./assets/../logo.svg" name: "cycle-skill".to_string(), description: "still loads".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::User, @@ -1100,6 +1283,7 @@ icon_large = "./assets/../logo.svg" name: "admin-linked-skill".to_string(), description: "from link".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&shared_skill_path), scope: SkillScope::Admin, @@ -1138,6 +1322,7 @@ icon_large = "./assets/../logo.svg" name: "repo-linked-skill".to_string(), description: "from link".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&linked_skill_path), scope: SkillScope::Repo, @@ -1199,6 +1384,7 @@ icon_large = "./assets/../logo.svg" name: "within-depth-skill".to_string(), description: "loads".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&within_depth_path), scope: SkillScope::User, @@ -1224,6 +1410,7 @@ icon_large = "./assets/../logo.svg" name: "demo-skill".to_string(), description: "does things carefully".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::User, @@ -1253,6 +1440,7 @@ icon_large = "./assets/../logo.svg" name: "demo-skill".to_string(), description: "long description".to_string(), short_description: Some("short summary".to_string()), + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::User, @@ -1363,6 +1551,7 @@ icon_large = "./assets/../logo.svg" name: "repo-skill".to_string(), description: "from repo".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, @@ -1414,6 +1603,7 @@ icon_large = "./assets/../logo.svg" name: "nested-skill".to_string(), description: "from nested".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&nested_skill_path), scope: SkillScope::Repo, @@ -1422,6 +1612,7 @@ icon_large = "./assets/../logo.svg" name: "root-skill".to_string(), description: "from root".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&root_skill_path), scope: SkillScope::Repo, @@ -1459,6 +1650,7 @@ icon_large = "./assets/../logo.svg" name: "local-skill".to_string(), description: "from cwd".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, @@ -1494,6 +1686,7 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from repo".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, @@ -1533,6 +1726,7 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from repo".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&repo_skill_path), scope: SkillScope::Repo, @@ -1541,6 +1735,7 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from user".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&user_skill_path), scope: SkillScope::User, @@ -1603,6 +1798,7 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: first_description.to_string(), short_description: None, + dependencies: None, interface: None, path: first_path, scope: SkillScope::Repo, @@ -1611,6 +1807,7 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: second_description.to_string(), short_description: None, + dependencies: None, interface: None, path: second_path, scope: SkillScope::Repo, @@ -1680,6 +1877,7 @@ icon_large = "./assets/../logo.svg" name: "repo-skill".to_string(), description: "from repo".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, @@ -1736,6 +1934,7 @@ icon_large = "./assets/../logo.svg" name: "system-skill".to_string(), description: "from system".to_string(), short_description: None, + dependencies: None, interface: None, path: normalized(&skill_path), scope: SkillScope::System, diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index cf7c180502..1c10ffaaab 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -1,3 +1,4 @@ +pub mod dependencies; pub mod injection; pub mod loader; pub mod manager; @@ -5,6 +6,7 @@ pub mod model; pub mod render; pub mod system; +pub(crate) use dependencies::handle_skill_dependencies; pub(crate) use injection::SkillInjections; pub(crate) use injection::build_skill_injections; pub use loader::load_skills; diff --git a/codex-rs/core/src/skills/model.rs b/codex-rs/core/src/skills/model.rs index fe3357f9d9..c46cd8d8a3 100644 --- a/codex-rs/core/src/skills/model.rs +++ b/codex-rs/core/src/skills/model.rs @@ -9,6 +9,7 @@ pub struct SkillMetadata { pub description: String, pub short_description: Option, pub interface: Option, + pub dependencies: Option, pub path: PathBuf, pub scope: SkillScope, } @@ -23,6 +24,20 @@ pub struct SkillInterface { pub default_prompt: Option, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillDependencies { + pub tools: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillToolDependency { + pub tool_type: String, + pub value: String, + pub description: Option, + pub transport: Option, + pub url: Option, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillError { pub path: PathBuf, diff --git a/codex-rs/core/src/tools/handlers/read_file.rs b/codex-rs/core/src/tools/handlers/read_file.rs index eb5ce647ce..7e6016e01f 100644 --- a/codex-rs/core/src/tools/handlers/read_file.rs +++ b/codex-rs/core/src/tools/handlers/read_file.rs @@ -1,11 +1,16 @@ use std::collections::VecDeque; +use std::path::Path; use std::path::PathBuf; use async_trait::async_trait; use codex_utils_string::take_bytes_at_char_boundary; +use dunce::canonicalize as canonicalize_path; use serde::Deserialize; +use crate::codex::Session; +use crate::codex::TurnContext; use crate::function_tool::FunctionCallError; +use crate::skills::handle_skill_dependencies; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -17,6 +22,7 @@ pub struct ReadFileHandler; const MAX_LINE_LENGTH: usize = 500; const TAB_WIDTH: usize = 4; +const SKILL_FILE_NAME: &str = "SKILL.md"; // TODO(jif) add support for block comments const COMMENT_PREFIXES: &[&str] = &["#", "//", "--"]; @@ -98,7 +104,13 @@ impl ToolHandler for ReadFileHandler { } async fn handle(&self, invocation: ToolInvocation) -> Result { - let ToolInvocation { payload, .. } = invocation; + let ToolInvocation { + session, + turn, + call_id, + payload, + .. + } = invocation; let arguments = match payload { ToolPayload::Function { arguments } => arguments, @@ -145,6 +157,8 @@ impl ToolHandler for ReadFileHandler { indentation::read_block(&path, offset, limit, indentation).await? } }; + maybe_prompt_missing_skill_dependencies(session.as_ref(), turn.as_ref(), &call_id, &path) + .await; Ok(ToolOutput::Function { content: collected.join("\n"), content_items: None, @@ -438,6 +452,37 @@ fn format_line(bytes: &[u8]) -> String { } } +async fn maybe_prompt_missing_skill_dependencies( + session: &Session, + turn: &TurnContext, + call_id: &str, + path: &Path, +) { + if !path + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.eq_ignore_ascii_case(SKILL_FILE_NAME)) + { + return; + } + + let outcome = session + .services + .skills_manager + .skills_for_cwd(&turn.cwd, false) + .await; + let resolved_path = canonicalize_path(path).unwrap_or_else(|_| path.to_path_buf()); + let Some(skill) = outcome + .skills + .iter() + .find(|skill| skill.path == resolved_path) + else { + return; + }; + + let _ = handle_skill_dependencies(session, turn, format!("{call_id}-mcp-deps"), skill).await; +} + fn trim_empty_lines(out: &mut VecDeque<&LineRecord>) { while matches!(out.front(), Some(line) if line.raw.trim().is_empty()) { out.pop_front(); diff --git a/codex-rs/docs/protocol_v1.md b/codex-rs/docs/protocol_v1.md index f5579008b2..1c7423150e 100644 --- a/codex-rs/docs/protocol_v1.md +++ b/codex-rs/docs/protocol_v1.md @@ -76,6 +76,7 @@ For complete documentation of the `Op` and `EventMsg` variants, refer to [protoc - `EventMsg::AgentMessage` – Messages from the `Model` - `EventMsg::ExecApprovalRequest` – Request approval from user to execute a command - `EventMsg::RequestUserInput` – Request user input for a tool call + - `EventMsg::SkillDependenciesApprovalRequest` – Prompt to install missing skill MCP dependencies - `EventMsg::TurnComplete` – A turn completed successfully - `EventMsg::Error` – A turn stopped with an error - `EventMsg::Warning` – A non-fatal warning that the client should surface to the user diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index c4a5c27d9c..d78e0a2864 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -607,7 +607,9 @@ impl EventProcessor for EventProcessorWithHumanOutput { | EventMsg::UndoCompleted(_) | EventMsg::UndoStarted(_) | EventMsg::ThreadRolledBack(_) - | EventMsg::RequestUserInput(_) => {} + | EventMsg::RequestUserInput(_) + | EventMsg::SkillDependenciesApprovalRequest(_) + | EventMsg::SkillRead(_) => {} } CodexStatus::Running } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index b400b4cc79..a2a2ed0f1a 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -360,6 +360,8 @@ async fn run_codex_tool_session_inner( | EventMsg::UndoCompleted(_) | EventMsg::ExitedReviewMode(_) | EventMsg::RequestUserInput(_) + | EventMsg::SkillDependenciesApprovalRequest(_) + | EventMsg::SkillRead(_) | EventMsg::ContextCompacted(_) | EventMsg::ThreadRolledBack(_) | EventMsg::CollabAgentSpawnBegin(_) diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 78050dfa86..f75c5c8ccd 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -93,3 +93,29 @@ pub struct ApplyPatchApprovalRequestEvent { #[serde(skip_serializing_if = "Option::is_none")] pub grant_root: Option, } + +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct SkillDependenciesApprovalOption { + pub label: String, + pub description: String, +} + +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct SkillDependenciesApprovalRequestEvent { + /// Identifier for the associated dependency prompt, if available. + pub call_id: String, + /// Turn ID that this request belongs to. + /// Uses `#[serde(default)]` for backwards compatibility. + #[serde(default)] + pub turn_id: String, + /// Question ID used to correlate the response. + pub question_id: String, + /// Header to display for the prompt. + pub header: String, + /// Prompt text describing the missing dependencies. + pub question: String, + /// Option to proceed without installing dependencies. + pub run_anyway: SkillDependenciesApprovalOption, + /// Option to install missing dependencies. + pub install: SkillDependenciesApprovalOption, +} diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 7b1c39d061..308a9eb6ca 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -47,6 +47,7 @@ pub use crate::approvals::ApplyPatchApprovalRequestEvent; pub use crate::approvals::ElicitationAction; pub use crate::approvals::ExecApprovalRequestEvent; pub use crate::approvals::ExecPolicyAmendment; +pub use crate::approvals::SkillDependenciesApprovalRequestEvent; pub use crate::request_user_input::RequestUserInputEvent; /// Open/close tags for special user-input blocks. Used across crates to avoid @@ -746,12 +747,17 @@ pub enum EventMsg { /// Notification that the agent attached a local image via the view_image tool. ViewImageToolCall(ViewImageToolCallEvent), + /// Notification that the read_file tool read a registered SKILL.md file. + SkillRead(SkillReadEvent), + ExecApprovalRequest(ExecApprovalRequestEvent), RequestUserInput(RequestUserInputEvent), ElicitationRequest(ElicitationRequestEvent), + SkillDependenciesApprovalRequest(SkillDependenciesApprovalRequestEvent), + ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent), /// Notification advising the user that something they are using has been @@ -1845,6 +1851,14 @@ pub struct ViewImageToolCallEvent { pub path: PathBuf, } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct SkillReadEvent { + /// Identifier for the originating tool call. + pub call_id: String, + /// Skill metadata associated with the read SKILL.md file. + pub skill: SkillMetadata, +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)] #[serde(rename_all = "snake_case")] pub enum ExecOutputStream { diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index e772a60215..21068e88b6 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1101,6 +1101,7 @@ mod tests { description: "test skill".to_string(), short_description: None, interface: None, + dependencies: None, path: PathBuf::from("test-skill"), scope: SkillScope::User, }]), diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a0f5821d65..a75b30602b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -92,12 +92,15 @@ use codex_otel::OtelManager; use codex_protocol::ThreadId; use codex_protocol::account::PlanType; use codex_protocol::approvals::ElicitationRequestEvent; +use codex_protocol::approvals::SkillDependenciesApprovalRequestEvent; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::models::local_image_label_text; use codex_protocol::parse_command::ParsedCommand; use codex_protocol::request_user_input::RequestUserInputEvent; +use codex_protocol::request_user_input::RequestUserInputQuestion; +use codex_protocol::request_user_input::RequestUserInputQuestionOption; use codex_protocol::user_input::TextElement; use codex_protocol::user_input::UserInput; use crossterm::event::KeyCode; @@ -265,6 +268,30 @@ fn is_standard_tool_call(parsed_cmd: &[ParsedCommand]) -> bool { .all(|parsed| !matches!(parsed, ParsedCommand::Unknown { .. })) } +fn skill_dependencies_to_user_input_request( + ev: SkillDependenciesApprovalRequestEvent, +) -> RequestUserInputEvent { + RequestUserInputEvent { + call_id: ev.call_id, + turn_id: ev.turn_id, + questions: vec![RequestUserInputQuestion { + id: ev.question_id, + header: ev.header, + question: ev.question, + options: Some(vec![ + RequestUserInputQuestionOption { + label: ev.run_anyway.label, + description: ev.run_anyway.description, + }, + RequestUserInputQuestionOption { + label: ev.install.label, + description: ev.install.description, + }, + ]), + }], + } +} + const RATE_LIMIT_WARNING_THRESHOLDS: [f64; 3] = [75.0, 90.0, 95.0]; const NUDGE_MODEL_SLUG: &str = "gpt-5.1-codex-mini"; const RATE_LIMIT_SWITCH_PROMPT_THRESHOLD: f64 = 90.0; @@ -1312,6 +1339,14 @@ impl ChatWidget { ); } + fn on_skill_dependencies_approval_request( + &mut self, + ev: SkillDependenciesApprovalRequestEvent, + ) { + let request = skill_dependencies_to_user_input_request(ev); + self.on_request_user_input(request); + } + fn on_exec_command_begin(&mut self, ev: ExecCommandBeginEvent) { self.flush_answer_stream_with_separator(); if is_unified_exec_source(ev.source) { @@ -2959,6 +2994,9 @@ impl ChatWidget { EventMsg::RequestUserInput(ev) => { self.on_request_user_input(ev); } + EventMsg::SkillDependenciesApprovalRequest(ev) => { + self.on_skill_dependencies_approval_request(ev); + } EventMsg::ExecCommandBegin(ev) => self.on_exec_command_begin(ev), EventMsg::TerminalInteraction(delta) => self.on_terminal_interaction(delta), EventMsg::ExecCommandOutputDelta(delta) => self.on_exec_command_output_delta(delta), @@ -3019,7 +3057,8 @@ impl ChatWidget { | EventMsg::ItemCompleted(_) | EventMsg::AgentMessageContentDelta(_) | EventMsg::ReasoningContentDelta(_) - | EventMsg::ReasoningRawContentDelta(_) => {} + | EventMsg::ReasoningRawContentDelta(_) + | EventMsg::SkillRead(_) => {} } } diff --git a/codex-rs/tui/src/chatwidget/skills.rs b/codex-rs/tui/src/chatwidget/skills.rs index d72ca60455..13e09dccbf 100644 --- a/codex-rs/tui/src/chatwidget/skills.rs +++ b/codex-rs/tui/src/chatwidget/skills.rs @@ -168,6 +168,7 @@ fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> SkillMetadata { brand_color: interface.brand_color, default_prompt: interface.default_prompt, }), + dependencies: None, path: skill.path.clone(), scope: skill.scope, }