[codex] remove EnvironmentPathRef (#27433)

We're switching to using a static encoding of the host path in
`PathUri`. We may need a type like this again but we can add it when
it's more compelling.

Stacked on #27454.
This commit is contained in:
Adam Perry @ OpenAI
2026-06-11 11:26:12 -07:00
committed by GitHub
Unverified
parent 0b08bbb9d9
commit 4a05d3b282
5 changed files with 34 additions and 602 deletions
+21 -6
View File
@@ -1,5 +1,5 @@
use codex_core_skills::model::SkillDependencies;
use codex_exec_server::EnvironmentPathRef;
use codex_utils_absolute_path::AbsolutePathBuf;
/// Source authority that owns a skill package and must be used to read it.
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
@@ -62,7 +62,7 @@ pub struct SkillPackageId(pub String);
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct SkillResourceId {
id: String,
environment_path: Option<EnvironmentPathRef>,
environment_path: Option<EnvironmentSkillResource>,
}
impl SkillResourceId {
@@ -73,10 +73,17 @@ impl SkillResourceId {
}
}
pub fn environment(id: impl Into<String>, path: EnvironmentPathRef) -> Self {
pub fn environment(
id: impl Into<String>,
environment_id: impl Into<String>,
path: AbsolutePathBuf,
) -> Self {
Self {
id: id.into(),
environment_path: Some(path),
environment_path: Some(EnvironmentSkillResource {
environment_id: environment_id.into(),
path,
}),
}
}
@@ -84,11 +91,19 @@ impl SkillResourceId {
&self.id
}
pub(crate) fn environment_path(&self) -> Option<&EnvironmentPathRef> {
self.environment_path.as_ref()
pub(crate) fn environment_path(&self) -> Option<(&str, &AbsolutePathBuf)> {
self.environment_path
.as_ref()
.map(|resource| (resource.environment_id.as_str(), &resource.path))
}
}
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
struct EnvironmentSkillResource {
environment_id: String,
path: AbsolutePathBuf,
}
/// Metadata shown in the always-visible skills catalog.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct SkillCatalogEntry {
+13 -7
View File
@@ -6,7 +6,6 @@ use codex_core_skills::filter_skill_load_outcome_for_product;
use codex_core_skills::loader::SkillRoot;
use codex_core_skills::loader::load_skills_from_roots;
use codex_exec_server::EnvironmentManager;
use codex_exec_server::EnvironmentPathRef;
use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
@@ -99,7 +98,7 @@ impl SkillProvider for ExecutorSkillProvider {
enabled,
authority.clone(),
&selected_root_id,
Arc::clone(&file_system),
&environment_id,
));
}
}
@@ -121,13 +120,19 @@ impl SkillProvider for ExecutorSkillProvider {
"executor skill resource does not match its package",
));
}
let Some(resource_path) = request.resource.environment_path() else {
let Some((environment_id, resource_path)) = request.resource.environment_path() else {
return Err(SkillProviderError::new(
"executor skill resource is not bound to an environment",
));
};
let contents = resource_path
.read_to_string(/*sandbox*/ None)
let Some(environment) = self.environment_manager.get_environment(environment_id) else {
return Err(SkillProviderError::new(format!(
"executor skill resource references unavailable environment `{environment_id}`"
)));
};
let contents = environment
.get_filesystem()
.read_file_text(resource_path, /*sandbox*/ None)
.await
.map_err(|err| {
SkillProviderError::new(format!(
@@ -153,7 +158,7 @@ fn catalog_entry_from_skill(
enabled: bool,
authority: SkillAuthority,
selected_root_id: &str,
file_system: Arc<dyn codex_exec_server::ExecutorFileSystem>,
environment_id: &str,
) -> SkillCatalogEntry {
let skill_path = skill.path_to_skills_md.to_string_lossy().into_owned();
let normalized_path = skill_path.replace('\\', "/");
@@ -168,7 +173,8 @@ fn catalog_entry_from_skill(
skill.description.clone(),
SkillResourceId::environment(
display_path.clone(),
EnvironmentPathRef::new(file_system, skill.path_to_skills_md.clone()),
environment_id,
skill.path_to_skills_md.clone(),
),
)
.with_short_description(skill.short_description.clone())
@@ -21,10 +21,8 @@ use codex_protocol::capabilities::CapabilityRootLocation;
use codex_protocol::capabilities::SelectedCapabilityRoot;
use codex_protocol::protocol::SkillScope;
use codex_skills_extension::ExecutorSkillProvider;
use codex_skills_extension::catalog::SkillReadResult;
use codex_skills_extension::provider::SkillListQuery;
use codex_skills_extension::provider::SkillProvider;
use codex_skills_extension::provider::SkillReadRequest;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
@@ -202,64 +200,6 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() {
);
}
#[tokio::test]
async fn executor_provider_reads_from_the_environment_instance_used_for_listing() {
let test_root = create_local_skill_root("bound-instance").expect("create local skill root");
let root_path = test_root.to_string_lossy().into_owned();
let environment_manager = Arc::new(EnvironmentManager::default_for_tests());
let provider = ExecutorSkillProvider::new_with_restriction_product(
Arc::clone(&environment_manager),
/*restriction_product*/ None,
);
let catalog = provider
.list(SkillListQuery {
turn_id: "turn-1".to_string(),
executor_roots: vec![SelectedCapabilityRoot {
id: "root-a".to_string(),
location: CapabilityRootLocation::Environment {
environment_id: "local".to_string(),
path: root_path,
},
}],
host: None,
include_host_skills: false,
include_bundled_skills: true,
include_orchestrator_skills: false,
mcp_resources: None,
})
.await
.expect("list executor skills");
let entry = catalog
.entries
.into_iter()
.next()
.expect("listed executor skill");
let resource = entry.main_prompt.clone();
environment_manager
.upsert_environment("local".to_string(), "http://127.0.0.1:1".to_string())
.expect("replace environment");
assert_eq!(
provider
.read(SkillReadRequest {
authority: entry.authority,
package: entry.id,
resource: resource.clone(),
host: None,
mcp_resources: None,
})
.await
.expect("read bound executor skill"),
SkillReadResult {
resource,
contents: SKILL_CONTENTS.to_string(),
}
);
std::fs::remove_dir_all(test_root).expect("remove skill directory");
}
#[tokio::test]
async fn selected_root_id_distinguishes_identical_executor_paths() {
let test_root = create_local_skill_root("root-identity").expect("create local skill root");