mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
Fix multi-env skill loader integrations
This commit is contained in:
@@ -518,6 +518,9 @@ impl CatalogRequestProcessor {
|
||||
let skills_manager = self.thread_manager.skills_manager();
|
||||
let plugins_manager = self.thread_manager.plugins_manager();
|
||||
let environment_manager = self.thread_manager.environment_manager();
|
||||
let selected_environment_id = environment_id
|
||||
.clone()
|
||||
.unwrap_or_else(|| codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string());
|
||||
let selected_environment = match environment_id {
|
||||
Some(environment_id) => {
|
||||
let environment = environment_manager
|
||||
@@ -539,6 +542,7 @@ impl CatalogRequestProcessor {
|
||||
let config = &config;
|
||||
let local_file_system = local_file_system.clone();
|
||||
let selected_environment = selected_environment.clone();
|
||||
let selected_environment_id = selected_environment_id.clone();
|
||||
let plugins_manager = &plugins_manager;
|
||||
let skills_manager = &skills_manager;
|
||||
async move {
|
||||
@@ -584,10 +588,13 @@ impl CatalogRequestProcessor {
|
||||
);
|
||||
};
|
||||
let skills_input = codex_core::skills::SkillsLoadInput::new(
|
||||
codex_exec_server::EnvironmentPathRef::new(
|
||||
environment.get_filesystem(),
|
||||
cwd_abs.clone(),
|
||||
),
|
||||
vec![codex_core::skills::loader::SkillEnvironment {
|
||||
environment_id: selected_environment_id.clone(),
|
||||
path: codex_exec_server::EnvironmentPathRef::new(
|
||||
environment.get_filesystem(),
|
||||
cwd_abs.clone(),
|
||||
),
|
||||
}],
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
config_layer_stack,
|
||||
|
||||
@@ -111,7 +111,10 @@ impl SkillsWatcher {
|
||||
let plugins_manager = thread_manager.plugins_manager();
|
||||
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
|
||||
let skills_input = SkillsLoadInput::new(
|
||||
root_path_ref,
|
||||
vec![codex_core::skills::loader::SkillEnvironment {
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
path: root_path_ref,
|
||||
}],
|
||||
local_file_system,
|
||||
plugin_outcome.effective_plugin_skill_roots(),
|
||||
config.config_layer_stack.clone(),
|
||||
|
||||
@@ -153,6 +153,7 @@ fn local_env_path(path: AbsolutePathBuf) -> EnvironmentPathRef {
|
||||
fn local_skill_root(path: AbsolutePathBuf, scope: SkillScope) -> SkillRoot {
|
||||
SkillRoot {
|
||||
path: local_env_path(path),
|
||||
environment_id: "local".to_string(),
|
||||
scope,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
@@ -1749,6 +1750,48 @@ async fn deduplicates_by_path_preferring_first_root() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn preserves_same_absolute_skill_path_from_distinct_environments() {
|
||||
let root = tempfile::tempdir().expect("tempdir");
|
||||
let skill_path = write_skill_at(root.path(), "dupe", "dupe-skill", "from repo");
|
||||
let shared_path = root.path().abs();
|
||||
let first_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
let second_file_system: Arc<dyn ExecutorFileSystem> = Arc::new(LocalFileSystem::unsandboxed());
|
||||
|
||||
let outcome = load_skills_from_roots([
|
||||
SkillRoot {
|
||||
path: EnvironmentPathRef::new(first_file_system, shared_path.clone()),
|
||||
environment_id: "devbox".to_string(),
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
},
|
||||
SkillRoot {
|
||||
path: EnvironmentPathRef::new(second_file_system, shared_path),
|
||||
environment_id: "local".to_string(),
|
||||
scope: SkillScope::Repo,
|
||||
plugin_id: None,
|
||||
plugin_root: None,
|
||||
},
|
||||
])
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
outcome
|
||||
.skills
|
||||
.iter()
|
||||
.map(|skill| (
|
||||
skill.environment_id.as_str(),
|
||||
skill.path_to_skills_md.clone()
|
||||
))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
("devbox", normalized(&skill_path)),
|
||||
("local", normalized(&skill_path)),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn keeps_duplicate_names_from_repo_and_user() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -427,6 +427,7 @@ enabled = false
|
||||
);
|
||||
let skills_input = skills_load_input_from_config(
|
||||
&config,
|
||||
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd,
|
||||
Some(Arc::clone(&codex_exec_server::LOCAL_FS)),
|
||||
effective_skill_roots,
|
||||
|
||||
@@ -452,16 +452,19 @@ async fn warm_plugins_and_skills_for_session_init(
|
||||
&environments,
|
||||
)
|
||||
.ok();
|
||||
let path_ref = resolved_environments
|
||||
let skill_environment = resolved_environments
|
||||
.as_ref()
|
||||
.and_then(crate::environment_selection::ResolvedTurnEnvironments::primary)
|
||||
.map(|environment| {
|
||||
crate::skills::EnvironmentPathRef::new(
|
||||
environment.environment.get_filesystem(),
|
||||
environment.cwd.clone(),
|
||||
(
|
||||
environment.environment_id.clone(),
|
||||
crate::skills::EnvironmentPathRef::new(
|
||||
environment.environment.get_filesystem(),
|
||||
environment.cwd.clone(),
|
||||
),
|
||||
)
|
||||
});
|
||||
let Some(path_ref) = path_ref else {
|
||||
let Some((environment_id, path_ref)) = skill_environment else {
|
||||
return Vec::new();
|
||||
};
|
||||
let local_file_system = environment_manager
|
||||
@@ -472,6 +475,7 @@ async fn warm_plugins_and_skills_for_session_init(
|
||||
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
|
||||
let skills_input = skills_load_input_from_config(
|
||||
config.as_ref(),
|
||||
environment_id,
|
||||
path_ref,
|
||||
local_file_system,
|
||||
effective_skill_roots,
|
||||
|
||||
@@ -4129,6 +4129,7 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() {
|
||||
.skills_for_cwd(
|
||||
&crate::skills_load_input_from_config(
|
||||
&parent_config,
|
||||
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
crate::skills::EnvironmentPathRef::new(
|
||||
Arc::clone(&skill_fs),
|
||||
parent_config.cwd.clone(),
|
||||
@@ -4709,6 +4710,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
);
|
||||
let skills_input = crate::skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd,
|
||||
services
|
||||
.environment_manager
|
||||
@@ -6562,6 +6564,7 @@ where
|
||||
);
|
||||
let skills_input = crate::skills_load_input_from_config(
|
||||
&per_turn_config,
|
||||
codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd,
|
||||
services
|
||||
.environment_manager
|
||||
|
||||
@@ -38,13 +38,14 @@ pub use codex_core_skills::system;
|
||||
|
||||
pub(crate) fn skills_load_input_from_config(
|
||||
config: &Config,
|
||||
environment_id: String,
|
||||
env_path: EnvironmentPathRef,
|
||||
local_file_system: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
) -> SkillsLoadInput {
|
||||
SkillsLoadInput::new(
|
||||
vec![codex_core_skills::loader::SkillEnvironment {
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
environment_id,
|
||||
path: env_path,
|
||||
}],
|
||||
local_file_system,
|
||||
|
||||
Reference in New Issue
Block a user