Compare commits

...

1 Commits

Author SHA1 Message Date
Xin Lin
e0320de5cb cleanup: Remove skill env var dependency prompting 2026-05-14 16:25:35 -07:00
12 changed files with 14 additions and 231 deletions

View File

@@ -1,30 +0,0 @@
use crate::SkillMetadata;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SkillDependencyInfo {
pub skill_name: String,
pub name: String,
pub description: Option<String>,
}
pub 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" || tool.value.is_empty() {
continue;
}
dependencies.push(SkillDependencyInfo {
skill_name: skill.name.clone(),
name: tool.value.clone(),
description: tool.description.clone(),
});
}
}
dependencies
}

View File

@@ -1,5 +1,4 @@
pub mod config_rules;
mod env_var_dependencies;
pub mod injection;
pub(crate) mod invocation_utils;
pub mod loader;
@@ -10,8 +9,6 @@ pub mod remote;
pub mod render;
pub mod system;
pub use env_var_dependencies::SkillDependencyInfo;
pub use env_var_dependencies::collect_env_var_dependencies;
pub(crate) use invocation_utils::build_implicit_skill_path_indexes;
pub use invocation_utils::detect_implicit_skill_invocation_for_command;
pub use manager::SkillsLoadInput;

View File

@@ -806,6 +806,14 @@ fn resolve_policy(policy: Option<Policy>) -> Option<SkillPolicy> {
}
fn resolve_dependency_tool(tool: DependencyTool) -> Option<SkillToolDependency> {
if tool
.kind
.as_deref()
.is_some_and(|kind| sanitize_single_line(kind).eq_ignore_ascii_case("env_var"))
{
return None;
}
let r#type = resolve_required_str(
tool.kind,
MAX_DEPENDENCY_TYPE_LEN,

View File

@@ -378,7 +378,7 @@ fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf {
}
#[tokio::test]
async fn loads_skill_dependencies_metadata_from_yaml() {
async fn loads_skill_dependencies_metadata_and_ignores_env_vars() {
let codex_home = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill(&codex_home, "demo", "dep-skill", "from json");
let skill_dir = skill_path.parent().expect("skill dir");
@@ -394,6 +394,10 @@ async fn loads_skill_dependencies_metadata_from_yaml() {
"value": "GITHUB_TOKEN",
"description": "GitHub API token with repo scopes"
},
{
"type": "env_var",
"description": "Missing value should still be ignored"
},
{
"type": "mcp",
"value": "github",
@@ -436,14 +440,6 @@ async fn loads_skill_dependencies_metadata_from_yaml() {
interface: None,
dependencies: Some(SkillDependencies {
tools: vec![
SkillToolDependency {
r#type: "env_var".to_string(),
value: "GITHUB_TOKEN".to_string(),
description: Some("GitHub API token with repo scopes".to_string()),
transport: None,
command: None,
url: None,
},
SkillToolDependency {
r#type: "mcp".to_string(),
value: "github".to_string(),

View File

@@ -547,9 +547,6 @@
"shell_zsh_fork": {
"type": "boolean"
},
"skill_env_var_dependency_prompt": {
"type": "boolean"
},
"skill_mcp_dependency_install": {
"type": "boolean"
},
@@ -4281,9 +4278,6 @@
"shell_zsh_fork": {
"type": "boolean"
},
"skill_env_var_dependency_prompt": {
"type": "boolean"
},
"skill_mcp_dependency_install": {
"type": "boolean"
},

View File

@@ -90,13 +90,11 @@ pub(crate) use skills::SkillsManager;
pub(crate) use skills::build_available_skills;
pub(crate) use skills::build_skill_injections;
pub(crate) use skills::build_skill_name_counts;
pub(crate) use skills::collect_env_var_dependencies;
pub(crate) use skills::collect_explicit_skill_mentions;
pub(crate) use skills::default_skill_metadata_budget;
pub(crate) use skills::injection;
pub(crate) use skills::manager;
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
pub(crate) use skills::resolve_skill_dependencies_for_turn;
pub(crate) use skills::skills_load_input_from_config;
mod stream_events_utils;
pub mod test_support;

View File

@@ -2989,16 +2989,6 @@ 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);

View File

@@ -9,7 +9,6 @@ use crate::build_skill_injections;
use crate::client::ModelClientSession;
use crate::client_common::Prompt;
use crate::client_common::ResponseEvent;
use crate::collect_env_var_dependencies;
use crate::collect_explicit_skill_mentions;
use crate::compact::InitialContextInjection;
use crate::compact::collect_user_messages;
@@ -39,7 +38,6 @@ use crate::mentions::collect_explicit_plugin_mentions;
use crate::mentions::collect_tool_mentions_from_messages;
use crate::parse_turn_item;
use crate::plugins::build_plugin_injections;
use crate::resolve_skill_dependencies_for_turn;
use crate::session::PreviousTurnSettings;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
@@ -227,15 +225,6 @@ pub(crate) async fn run_turn(
&connector_slug_counts,
)
});
let config = turn_context.config.clone();
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(),

View File

@@ -1,8 +1,3 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::env;
use std::sync::Arc;
use crate::config::Config;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
@@ -10,14 +5,9 @@ use codex_analytics::InvocationType;
use codex_analytics::SkillInvocation;
use codex_analytics::build_track_events_context;
use codex_protocol::protocol::SkillScope;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputQuestion;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::PluginSkillRoot;
use tracing::warn;
pub use codex_core_skills::SkillDependencyInfo;
pub use codex_core_skills::SkillError;
pub use codex_core_skills::SkillLoadOutcome;
pub use codex_core_skills::SkillMetadata;
@@ -27,7 +17,6 @@ pub use codex_core_skills::SkillsLoadInput;
pub use codex_core_skills::SkillsManager;
pub use codex_core_skills::build_available_skills;
pub use codex_core_skills::build_skill_name_counts;
pub use codex_core_skills::collect_env_var_dependencies;
pub use codex_core_skills::config_rules;
pub use codex_core_skills::default_skill_metadata_budget;
pub use codex_core_skills::detect_implicit_skill_invocation_for_command;
@@ -56,121 +45,6 @@ pub(crate) fn skills_load_input_from_config(
)
}
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()) || existing_env.contains_key(&name) {
continue;
}
match env::var(&name) {
Ok(value) => {
loaded_values.insert(name.clone(), value);
}
Err(env::VarError::NotPresent) => {
missing.push(dependency.clone());
}
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;
}
}
async fn request_skill_dependencies(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
dependencies: &[SkillDependencyInfo],
) {
let questions = dependencies
.iter()
.map(|dependency| {
let requirement = dependency.description.as_ref().map_or_else(
|| {
format!(
"The skill \"{}\" requires \"{}\" to be set.",
dependency.skill_name, dependency.name
)
},
|description| {
format!(
"The skill \"{}\" requires \"{}\" to be set ({}).",
dependency.skill_name, dependency.name, description
)
},
);
RequestUserInputQuestion {
id: dependency.name.clone(),
header: "Skill requires environment variable".to_string(),
question: format!(
"{requirement} This is an experimental internal feature. The value is stored in memory for this session only."
),
is_other: false,
is_secret: true,
options: None,
}
})
.collect::<Vec<_>>();
if questions.is_empty() {
return;
}
let response = sess
.request_user_input(
turn_context,
format!("skill-deps-{}", turn_context.sub_id),
RequestUserInputArgs { questions },
)
.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;
}
pub(crate) async fn maybe_emit_implicit_skill_invocation(
sess: &Session,
turn_context: &TurnContext,

View File

@@ -3,7 +3,6 @@
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::models::ResponseItem;
use codex_sandboxing::policy_transforms::merge_permission_profiles;
use std::collections::HashMap;
use std::collections::HashSet;
use crate::context_manager::ContextManager;
@@ -22,7 +21,6 @@ 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>,
/// Settings used by the latest regular user turn, used for turn-to-turn
/// model/realtime handling on subsequent regular turns (including full-context
@@ -45,7 +43,6 @@ impl SessionState {
history,
latest_rate_limits: None,
server_reasoning_included: false,
dependency_env: HashMap::new(),
mcp_dependency_prompted: HashSet::new(),
previous_turn_settings: None,
startup_prewarm: None,
@@ -165,16 +162,6 @@ impl SessionState {
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()
}
pub(crate) fn set_session_startup_prewarm(
&mut self,
startup_prewarm: SessionStartupPrewarmHandle,

View File

@@ -72,7 +72,6 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
shell_runtime_backend,
} = args;
let mut exec_params = exec_params;
let Some(turn_environment) = turn.environments.primary() else {
return Err(FunctionCallError::RespondToModel(
"shell is unavailable in this session".to_string(),
@@ -80,18 +79,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
};
let fs = turn_environment.environment.get_filesystem();
let dependency_env = session.dependency_env().await;
if !dependency_env.is_empty() {
exec_params.env.extend(dependency_env.clone());
}
let mut explicit_env_overrides = turn.shell_environment_policy.r#set.clone();
for key in dependency_env.keys() {
if let Some(value) = exec_params.env.get(key) {
explicit_env_overrides.insert(key.clone(), value.clone());
}
}
let explicit_env_overrides = turn.shell_environment_policy.r#set.clone();
let exec_permission_approvals_enabled =
session.features().enabled(Feature::ExecPermissionApprovals);
let requested_additional_permissions = additional_permissions.clone();

View File

@@ -196,8 +196,6 @@ pub enum Feature {
ImageGeneration,
/// Allow prompting and installing missing MCP dependencies.
SkillMcpDependencyInstall,
/// Prompt for missing skill env var dependencies.
SkillEnvVarDependencyPrompt,
/// Enable the unified mention popup prototype.
MentionsV2,
/// Steer feature flag - when enabled, Enter submits immediately instead of queuing.
@@ -1031,12 +1029,6 @@ 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::MentionsV2,
key: "mentions_v2",