mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
cleanup: Remove skill env var dependency prompting (#22721)
Deletes the skill env var dependency prompt feature and its runtime path. env_var entries in skill dependency metadata are now silently ignored during skill loading.
This commit is contained in:
committed by
GitHub
Unverified
parent
17d552fb4d
commit
6b54ced108
@@ -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
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -399,11 +399,6 @@ async fn loads_skill_dependencies_metadata_from_yaml() {
|
||||
{
|
||||
"dependencies": {
|
||||
"tools": [
|
||||
{
|
||||
"type": "env_var",
|
||||
"value": "GITHUB_TOKEN",
|
||||
"description": "GitHub API token with repo scopes"
|
||||
},
|
||||
{
|
||||
"type": "mcp",
|
||||
"value": "github",
|
||||
@@ -446,14 +441,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(),
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -3019,16 +3019,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);
|
||||
|
||||
@@ -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;
|
||||
@@ -40,7 +39,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;
|
||||
@@ -234,15 +232,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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -70,7 +70,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(),
|
||||
@@ -78,18 +77,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();
|
||||
|
||||
@@ -168,7 +168,7 @@ pub enum Feature {
|
||||
ImageGeneration,
|
||||
/// Allow prompting and installing missing MCP dependencies.
|
||||
SkillMcpDependencyInstall,
|
||||
/// Prompt for missing skill env var dependencies.
|
||||
/// Removed compatibility flag for deleted skill env var dependency prompting.
|
||||
SkillEnvVarDependencyPrompt,
|
||||
/// Enable the unified mention popup prototype.
|
||||
MentionsV2,
|
||||
@@ -431,6 +431,9 @@ impl Features {
|
||||
"image_detail_original" => {
|
||||
continue;
|
||||
}
|
||||
"skill_env_var_dependency_prompt" => {
|
||||
continue;
|
||||
}
|
||||
"use_legacy_landlock" => {
|
||||
self.record_legacy_usage_force(
|
||||
"features.use_legacy_landlock",
|
||||
@@ -1037,7 +1040,7 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::SkillEnvVarDependencyPrompt,
|
||||
key: "skill_env_var_dependency_prompt",
|
||||
stage: Stage::UnderDevelopment,
|
||||
stage: Stage::Removed,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
|
||||
Reference in New Issue
Block a user