From bdd8a7d58bdd4ea57efe17f24cde4c09fdfea0b0 Mon Sep 17 00:00:00 2001 From: xl-openai Date: Thu, 29 Jan 2026 11:13:30 -0800 Subject: [PATCH] Better handling skill depdenencies on ENV VAR. (#9017) An experimental flow for env var skill dependencies. Skills can now declare required env vars in SKILL.md; if missing, the CLI prompts the user to get the value, and Core will store it in memory (eventually to a local persistent store) image --- .../app-server-protocol/src/protocol/v2.rs | 2 + .../app-server/src/bespoke_event_handling.rs | 1 + codex-rs/core/config.schema.json | 6 + codex-rs/core/src/codex.rs | 21 +++ codex-rs/core/src/features.rs | 8 + codex-rs/core/src/mcp/skill_dependencies.rs | 1 + .../core/src/skills/env_var_dependencies.rs | 162 ++++++++++++++++++ codex-rs/core/src/skills/mod.rs | 3 + codex-rs/core/src/state/session.rs | 13 ++ codex-rs/core/src/tools/handlers/shell.rs | 6 + codex-rs/protocol/src/request_user_input.rs | 4 + codex-rs/tui/src/bottom_pane/chat_composer.rs | 13 +- .../src/bottom_pane/request_user_input/mod.rs | 6 + .../bottom_pane/request_user_input/render.rs | 9 +- codex-rs/tui/src/bottom_pane/textarea.rs | 36 ++++ 15 files changed, 289 insertions(+), 2 deletions(-) create mode 100644 codex-rs/core/src/skills/env_var_dependencies.rs diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 7b695d2c7..247360282 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -2532,6 +2532,8 @@ pub struct ToolRequestUserInputQuestion { pub question: String, #[serde(default)] pub is_other: bool, + #[serde(default)] + pub is_secret: bool, pub options: Option>, } diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 20e7db762..1a9fcaa70 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -279,6 +279,7 @@ pub(crate) async fn apply_bespoke_event_handling( header: question.header, question: question.question, is_other: question.is_other, + is_secret: question.is_secret, options: question.options.map(|options| { options .into_iter() diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 717eddc61..16c0c8945 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -207,6 +207,9 @@ "shell_tool": { "type": "boolean" }, + "skill_env_var_dependency_prompt": { + "type": "boolean" + }, "skill_mcp_dependency_install": { "type": "boolean" }, @@ -1231,6 +1234,9 @@ "shell_tool": { "type": "boolean" }, + "skill_env_var_dependency_prompt": { + "type": "boolean" + }, "skill_mcp_dependency_install": { "type": "boolean" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 736d0554b..1b7b7312c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -168,10 +168,12 @@ use crate::skills::SkillInjections; use crate::skills::SkillMetadata; use crate::skills::SkillsManager; use crate::skills::build_skill_injections; +use crate::skills::collect_env_var_dependencies; use crate::skills::collect_explicit_skill_mentions; use crate::skills::injection::ToolMentionKind; use crate::skills::injection::app_id_from_path; use crate::skills::injection::tool_kind_for_path; +use crate::skills::resolve_skill_dependencies_for_turn; use crate::state::ActiveTurn; use crate::state::SessionServices; use crate::state::SessionState; @@ -1953,6 +1955,16 @@ impl Session { state.record_mcp_dependency_prompted(names); } + pub async fn dependency_env(&self) -> HashMap { + let state = self.state.lock().await; + state.dependency_env() + } + + pub async fn set_dependency_env(&self, values: HashMap) { + let mut state = self.state.lock().await; + state.set_dependency_env(values); + } + pub(crate) async fn set_server_reasoning_included(&self, included: bool) { let mut state = self.state.lock().await; state.set_server_reasoning_included(included); @@ -3218,6 +3230,15 @@ pub(crate) async fn run_turn( }); let explicit_app_paths = collect_explicit_app_paths(&input); + let config = turn_context.client.config(); + if config + .features + .enabled(Feature::SkillEnvVarDependencyPrompt) + { + let env_var_dependencies = collect_env_var_dependencies(&mentioned_skills); + resolve_skill_dependencies_for_turn(&sess, &turn_context, &env_var_dependencies).await; + } + maybe_prompt_and_install_mcp_dependencies( sess.as_ref(), turn_context.as_ref(), diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 9bc52537b..961b7a56f 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -115,6 +115,8 @@ pub enum Feature { Apps, /// Allow prompting and installing missing MCP dependencies. SkillMcpDependencyInstall, + /// Prompt for missing skill env var dependencies. + SkillEnvVarDependencyPrompt, /// Steer feature flag - when enabled, Enter submits immediately instead of queuing. Steer, /// Enable collaboration modes (Plan, Code, Pair Programming, Execute). @@ -533,6 +535,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: true, }, + FeatureSpec { + id: Feature::SkillEnvVarDependencyPrompt, + key: "skill_env_var_dependency_prompt", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::Steer, key: "steer", diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index c295a9686..37620d73d 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -79,6 +79,7 @@ async fn should_install_mcp_dependencies( "The following MCP servers are required by the selected skills but are not installed yet: {server_list}. Install them now?" ), is_other: false, + is_secret: false, options: Some(vec![ RequestUserInputQuestionOption { label: MCP_DEPENDENCY_OPTION_INSTALL.to_string(), diff --git a/codex-rs/core/src/skills/env_var_dependencies.rs b/codex-rs/core/src/skills/env_var_dependencies.rs new file mode 100644 index 000000000..00f5bad8c --- /dev/null +++ b/codex-rs/core/src/skills/env_var_dependencies.rs @@ -0,0 +1,162 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::env; +use std::sync::Arc; + +use codex_protocol::request_user_input::RequestUserInputArgs; +use codex_protocol::request_user_input::RequestUserInputQuestion; +use codex_protocol::request_user_input::RequestUserInputResponse; +use tracing::warn; + +use crate::codex::Session; +use crate::codex::TurnContext; +use crate::skills::SkillMetadata; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct SkillDependencyInfo { + pub(crate) skill_name: String, + pub(crate) name: String, + pub(crate) description: Option, +} + +/// Resolve required dependency values (session cache, then env vars), +/// and prompt the UI for any missing ones. +pub(crate) async fn resolve_skill_dependencies_for_turn( + sess: &Arc, + turn_context: &Arc, + dependencies: &[SkillDependencyInfo], +) { + if dependencies.is_empty() { + return; + } + + let existing_env = sess.dependency_env().await; + let mut loaded_values = HashMap::new(); + let mut missing = Vec::new(); + let mut seen_names = HashSet::new(); + + for dependency in dependencies { + let name = dependency.name.clone(); + if !seen_names.insert(name.clone()) { + continue; + } + if existing_env.contains_key(&name) { + continue; + } + match env::var(&name) { + Ok(value) => { + loaded_values.insert(name.clone(), value); + continue; + } + Err(env::VarError::NotPresent) => {} + Err(err) => { + warn!("failed to read env var {name}: {err}"); + } + } + missing.push(dependency.clone()); + } + + if !loaded_values.is_empty() { + sess.set_dependency_env(loaded_values).await; + } + + if !missing.is_empty() { + request_skill_dependencies(sess, turn_context, &missing).await; + } +} + +pub(crate) fn collect_env_var_dependencies( + mentioned_skills: &[SkillMetadata], +) -> Vec { + let mut dependencies = Vec::new(); + for skill in mentioned_skills { + let Some(skill_dependencies) = &skill.dependencies else { + continue; + }; + for tool in &skill_dependencies.tools { + if tool.r#type != "env_var" { + continue; + } + if tool.value.is_empty() { + continue; + } + dependencies.push(SkillDependencyInfo { + skill_name: skill.name.clone(), + name: tool.value.clone(), + description: tool.description.clone(), + }); + } + } + dependencies +} + +/// Prompt via request_user_input to gather missing env vars. +pub(crate) async fn request_skill_dependencies( + sess: &Arc, + turn_context: &Arc, + dependencies: &[SkillDependencyInfo], +) { + let questions = dependencies + .iter() + .map(|dep| { + let requirement = dep.description.as_ref().map_or_else( + || format!("The skill \"{}\" requires \"{}\" to be set.", dep.skill_name, dep.name), + |description| { + format!( + "The skill \"{}\" requires \"{}\" to be set ({}).", + dep.skill_name, dep.name, description + ) + }, + ); + let question = format!( + "{requirement} This is an experimental internal feature. The value is stored in memory for this session only.", + ); + RequestUserInputQuestion { + id: dep.name.clone(), + header: "Skill requires environment variable".to_string(), + question, + is_other: false, + is_secret: true, + options: None, + } + }) + .collect::>(); + + if questions.is_empty() { + return; + } + + let args = RequestUserInputArgs { questions }; + let call_id = format!("skill-deps-{}", turn_context.sub_id); + let response = sess + .request_user_input(turn_context, call_id, args) + .await + .unwrap_or_else(|| RequestUserInputResponse { + answers: HashMap::new(), + }); + + if response.answers.is_empty() { + return; + } + + let mut values = HashMap::new(); + for (name, answer) in response.answers { + let mut user_note = None; + for entry in &answer.answers { + if let Some(note) = entry.strip_prefix("user_note: ") + && !note.trim().is_empty() + { + user_note = Some(note.trim().to_string()); + } + } + if let Some(value) = user_note { + values.insert(name, value); + } + } + + if values.is_empty() { + return; + } + + sess.set_dependency_env(values).await; +} diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index ae4ffa292..4d79bca25 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -1,3 +1,4 @@ +mod env_var_dependencies; pub mod injection; pub mod loader; pub mod manager; @@ -5,6 +6,8 @@ pub mod model; pub mod render; pub mod system; +pub(crate) use env_var_dependencies::collect_env_var_dependencies; +pub(crate) use env_var_dependencies::resolve_skill_dependencies_for_turn; pub(crate) use injection::SkillInjections; pub(crate) use injection::build_skill_injections; pub(crate) use injection::collect_explicit_skill_mentions; diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index 9d3d96db9..deee0d0c7 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -1,6 +1,7 @@ //! Session-wide mutable state. use codex_protocol::models::ResponseItem; +use std::collections::HashMap; use std::collections::HashSet; use crate::codex::SessionConfiguration; @@ -16,6 +17,7 @@ pub(crate) struct SessionState { pub(crate) history: ContextManager, pub(crate) latest_rate_limits: Option, pub(crate) server_reasoning_included: bool, + pub(crate) dependency_env: HashMap, pub(crate) mcp_dependency_prompted: HashSet, /// Whether the session's initial context has been seeded into history. /// @@ -33,6 +35,7 @@ impl SessionState { history, latest_rate_limits: None, server_reasoning_included: false, + dependency_env: HashMap::new(), mcp_dependency_prompted: HashSet::new(), initial_context_seeded: false, } @@ -112,6 +115,16 @@ impl SessionState { pub(crate) fn mcp_dependency_prompted(&self) -> HashSet { self.mcp_dependency_prompted.clone() } + + pub(crate) fn set_dependency_env(&mut self, values: HashMap) { + for (key, value) in values { + self.dependency_env.insert(key, value); + } + } + + pub(crate) fn dependency_env(&self) -> HashMap { + self.dependency_env.clone() + } } // Sometimes new snapshots don't include credits or plan information. diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index dbfb0364b..b59c0140d 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -233,6 +233,12 @@ impl ShellHandler { None }; + let mut exec_params = exec_params; + let dependency_env = session.dependency_env().await; + if !dependency_env.is_empty() { + exec_params.env.extend(dependency_env); + } + // Approval policy guard for explicit escalation in non-OnRequest modes. if exec_params .sandbox_permissions diff --git a/codex-rs/protocol/src/request_user_input.rs b/codex-rs/protocol/src/request_user_input.rs index 0e58348da..cb076264d 100644 --- a/codex-rs/protocol/src/request_user_input.rs +++ b/codex-rs/protocol/src/request_user_input.rs @@ -20,6 +20,10 @@ pub struct RequestUserInputQuestion { #[schemars(rename = "isOther")] #[ts(rename = "isOther")] pub is_other: bool, + #[serde(rename = "isSecret", default)] + #[schemars(rename = "isSecret")] + #[ts(rename = "isSecret")] + pub is_secret: bool, #[serde(skip_serializing_if = "Option::is_none")] pub options: Option>, } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 9d0cbf22e..90cae4b4a 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -2854,6 +2854,12 @@ impl Renderable for ChatComposer { } fn render(&self, area: Rect, buf: &mut Buffer) { + self.render_with_mask(area, buf, None); + } +} + +impl ChatComposer { + pub(crate) fn render_with_mask(&self, area: Rect, buf: &mut Buffer, mask_char: Option) { let [composer_rect, textarea_rect, popup_rect] = self.layout_areas(area); match &self.active_popup { ActivePopup::Command(popup) => { @@ -3018,7 +3024,12 @@ impl Renderable for ChatComposer { } let mut state = self.textarea_state.borrow_mut(); - StatefulWidgetRef::render_ref(&(&self.textarea), textarea_rect, buf, &mut state); + if let Some(mask_char) = mask_char { + self.textarea + .render_ref_masked(textarea_rect, buf, &mut state, mask_char); + } else { + StatefulWidgetRef::render_ref(&(&self.textarea), textarea_rect, buf, &mut state); + } if self.textarea.text().is_empty() { let text = if self.input_enabled { self.placeholder_text.as_str().to_string() diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs index 979499835..8fe340169 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs +++ b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs @@ -1251,6 +1251,7 @@ mod tests { header: header.to_string(), question: "Choose an option.".to_string(), is_other: false, + is_secret: false, options: Some(vec![ RequestUserInputQuestionOption { label: "Option 1".to_string(), @@ -1274,6 +1275,7 @@ mod tests { header: header.to_string(), question: "Choose an option.".to_string(), is_other: true, + is_secret: false, options: Some(vec![ RequestUserInputQuestionOption { label: "Option 1".to_string(), @@ -1297,6 +1299,7 @@ mod tests { header: header.to_string(), question: "Choose the next step for this task.".to_string(), is_other: false, + is_secret: false, options: Some(vec![ RequestUserInputQuestionOption { label: "Discuss a code change".to_string(), @@ -1326,6 +1329,7 @@ mod tests { header: header.to_string(), question: "Share details.".to_string(), is_other: false, + is_secret: false, options: None, } } @@ -2385,6 +2389,7 @@ mod tests { header: "Next Step".to_string(), question: "What would you like to do next?".to_string(), is_other: false, + is_secret: false, options: Some(vec![ RequestUserInputQuestionOption { label: "Discuss a code change (Recommended)".to_string(), @@ -2436,6 +2441,7 @@ mod tests { header: "Next Step".to_string(), question: "What would you like to do next?".to_string(), is_other: false, + is_secret: false, options: Some(vec![ RequestUserInputQuestionOption { label: "Discuss a code change (Recommended)".to_string(), diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/render.rs b/codex-rs/tui/src/bottom_pane/request_user_input/render.rs index b7433f8a6..2180ec055 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/render.rs +++ b/codex-rs/tui/src/bottom_pane/request_user_input/render.rs @@ -414,7 +414,14 @@ impl RequestUserInputOverlay { if area.width == 0 || area.height == 0 { return; } - self.composer.render(area, buf); + let is_secret = self + .current_question() + .is_some_and(|question| question.is_secret); + if is_secret { + self.composer.render_with_mask(area, buf, Some('*')); + } else { + self.composer.render(area, buf); + } } } diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 0235f7a9c..42f7b09be 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -1146,6 +1146,22 @@ impl StatefulWidgetRef for &TextArea { } impl TextArea { + pub(crate) fn render_ref_masked( + &self, + area: Rect, + buf: &mut Buffer, + state: &mut TextAreaState, + mask_char: char, + ) { + let lines = self.wrapped_lines(area.width); + let scroll = self.effective_scroll(area.height, &lines, state.scroll); + state.scroll = scroll; + + let start = scroll as usize; + let end = (scroll + area.height).min(lines.len() as u16) as usize; + self.render_lines_masked(area, buf, &lines, start..end, mask_char); + } + fn render_lines( &self, area: Rect, @@ -1175,6 +1191,26 @@ impl TextArea { } } } + + fn render_lines_masked( + &self, + area: Rect, + buf: &mut Buffer, + lines: &[Range], + range: std::ops::Range, + mask_char: char, + ) { + for (row, idx) in range.enumerate() { + let r = &lines[idx]; + let y = area.y + row as u16; + let line_range = r.start..r.end - 1; + let masked = self.text[line_range.clone()] + .chars() + .map(|_| mask_char) + .collect::(); + buf.set_string(area.x, y, &masked, Style::default()); + } + } } #[cfg(test)]