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) <img width="790" height="169" alt="image" src="https://github.com/user-attachments/assets/cd928918-9403-43cb-a7e7-b8d59bcccd9a" />
This commit is contained in:
parent
b7f26d74f0
commit
bdd8a7d58b
15 changed files with 289 additions and 2 deletions
|
|
@ -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<Vec<ToolRequestUserInputOption>>,
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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<String, String> {
|
||||
let state = self.state.lock().await;
|
||||
state.dependency_env()
|
||||
}
|
||||
|
||||
pub async fn set_dependency_env(&self, values: HashMap<String, String>) {
|
||||
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(),
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
162
codex-rs/core/src/skills/env_var_dependencies.rs
Normal file
162
codex-rs/core/src/skills/env_var_dependencies.rs
Normal file
|
|
@ -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<String>,
|
||||
}
|
||||
|
||||
/// 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<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
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<SkillDependencyInfo> {
|
||||
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<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
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::<Vec<_>>();
|
||||
|
||||
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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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<RateLimitSnapshot>,
|
||||
pub(crate) server_reasoning_included: bool,
|
||||
pub(crate) dependency_env: HashMap<String, String>,
|
||||
pub(crate) mcp_dependency_prompted: HashSet<String>,
|
||||
/// 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<String> {
|
||||
self.mcp_dependency_prompted.clone()
|
||||
}
|
||||
|
||||
pub(crate) fn set_dependency_env(&mut self, values: HashMap<String, String>) {
|
||||
for (key, value) in values {
|
||||
self.dependency_env.insert(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn dependency_env(&self) -> HashMap<String, String> {
|
||||
self.dependency_env.clone()
|
||||
}
|
||||
}
|
||||
|
||||
// Sometimes new snapshots don't include credits or plan information.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<Vec<RequestUserInputQuestionOption>>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<char>) {
|
||||
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()
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<usize>],
|
||||
range: std::ops::Range<usize>,
|
||||
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::<String>();
|
||||
buf.set_string(area.x, y, &masked, Style::default());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue