From 41c810bd3dc467d8213242c18c774f9512fda044 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Mon, 1 Jun 2026 20:33:35 +0000 Subject: [PATCH] core-skills: route reads through source paths --- codex-rs/core-skills/src/injection.rs | 9 +-- codex-rs/core-skills/src/invocation_utils.rs | 59 ++++++++++++------- codex-rs/core-skills/src/manager.rs | 6 +- codex-rs/core-skills/src/model.rs | 4 +- codex-rs/core/src/session/turn.rs | 1 - codex-rs/core/src/session/turn_context.rs | 3 +- codex-rs/core/src/skills.rs | 20 +++---- .../src/tools/handlers/shell/shell_command.rs | 6 ++ .../handlers/unified_exec/exec_command.rs | 3 +- 9 files changed, 61 insertions(+), 50 deletions(-) diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index 5dd704fec6..63142efb4a 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -1,7 +1,6 @@ use std::collections::HashMap; use std::collections::HashSet; -use crate::SkillLoadOutcome; use crate::SkillMetadata; use crate::mention_counts::build_skill_name_counts_for_raw_paths; use codex_analytics::AnalyticsEventsClient; @@ -28,7 +27,6 @@ pub struct SkillInjection { pub async fn build_skill_injections( mentioned_skills: &[SkillMetadata], - _loaded_skills: Option<&SkillLoadOutcome>, otel: Option<&SessionTelemetry>, analytics_client: &AnalyticsEventsClient, tracking: TrackEventsContext, @@ -44,12 +42,7 @@ pub async fn build_skill_injections( let mut invocations = Vec::new(); for skill in mentioned_skills { - match skill - .source_path - .file_system() - .read_file_text(&skill.path_to_skills_md, /*sandbox*/ None) - .await - { + match skill.source_path.read_to_string(/*sandbox*/ None).await { Ok(contents) => { emit_skill_injected_metric(otel, skill, "ok"); invocations.push(SkillInvocation { diff --git a/codex-rs/core-skills/src/invocation_utils.rs b/codex-rs/core-skills/src/invocation_utils.rs index 4c9d0a4119..448e8b9c9c 100644 --- a/codex-rs/core-skills/src/invocation_utils.rs +++ b/codex-rs/core-skills/src/invocation_utils.rs @@ -3,22 +3,25 @@ use std::path::Path; use crate::SkillLoadOutcome; use crate::SkillMetadata; +use codex_exec_server::EnvironmentPathRef; use codex_utils_absolute_path::AbsolutePathBuf; -pub(crate) fn build_implicit_skill_path_indexes( +pub(crate) async fn build_implicit_skill_path_indexes( skills: Vec, ) -> ( - HashMap, - HashMap, + HashMap, + HashMap, ) { let mut by_scripts_dir = HashMap::new(); let mut by_skill_doc_path = HashMap::new(); for skill in skills { - let skill_doc_path = canonicalize_if_exists(&skill.path_to_skills_md); + let skill_doc_path = canonicalize_if_exists(&skill.source_path).await; by_skill_doc_path.insert(skill_doc_path, skill.clone()); - if let Some(skill_dir) = skill.path_to_skills_md.parent() { - let scripts_dir = canonicalize_if_exists(&skill_dir.join("scripts")); + if let Ok(Some(skill_dir)) = skill.source_path.parent().await + && let Ok(scripts_dir) = skill_dir.join(Path::new("scripts")).await + { + let scripts_dir = canonicalize_if_exists(&scripts_dir).await; by_scripts_dir.insert(scripts_dir, skill); } } @@ -26,19 +29,19 @@ pub(crate) fn build_implicit_skill_path_indexes( (by_scripts_dir, by_skill_doc_path) } -pub fn detect_implicit_skill_invocation_for_command( +pub async fn detect_implicit_skill_invocation_for_command( outcome: &SkillLoadOutcome, command: &str, - workdir: &AbsolutePathBuf, + workdir: &EnvironmentPathRef, ) -> Option { - let workdir = canonicalize_if_exists(workdir); + let workdir = canonicalize_if_exists(workdir).await; let tokens = tokenize_command(command); - if let Some(candidate) = detect_skill_script_run(outcome, tokens.as_slice(), &workdir) { + if let Some(candidate) = detect_skill_script_run(outcome, tokens.as_slice(), &workdir).await { return Some(candidate); } - detect_skill_doc_read(outcome, tokens.as_slice(), &workdir) + detect_skill_doc_read(outcome, tokens.as_slice(), &workdir).await } fn tokenize_command(command: &str) -> Vec { @@ -78,28 +81,30 @@ fn script_run_token(tokens: &[String]) -> Option<&str> { None } -fn detect_skill_script_run( +async fn detect_skill_script_run( outcome: &SkillLoadOutcome, tokens: &[String], - workdir: &AbsolutePathBuf, + workdir: &EnvironmentPathRef, ) -> Option { let script_token = script_run_token(tokens)?; let script_path = Path::new(script_token); - let script_path = canonicalize_if_exists(&workdir.join(script_path)); + let script_path = canonicalize_if_exists(&resolve_path_ref(workdir, script_path).await?).await; - for path in script_path.ancestors() { - if let Some(candidate) = outcome.implicit_skills_by_scripts_dir.get(&path) { + let mut path = Some(script_path); + while let Some(candidate_path) = path { + if let Some(candidate) = outcome.implicit_skills_by_scripts_dir.get(&candidate_path) { return Some(candidate.clone()); } + path = candidate_path.parent().await.ok().flatten(); } None } -fn detect_skill_doc_read( +async fn detect_skill_doc_read( outcome: &SkillLoadOutcome, tokens: &[String], - workdir: &AbsolutePathBuf, + workdir: &EnvironmentPathRef, ) -> Option { if !command_reads_file(tokens) { return None; @@ -110,7 +115,7 @@ fn detect_skill_doc_read( continue; } let path = Path::new(token); - let candidate_path = canonicalize_if_exists(&workdir.join(path)); + let candidate_path = canonicalize_if_exists(&resolve_path_ref(workdir, path).await?).await; if let Some(candidate) = outcome.implicit_skills_by_doc_path.get(&candidate_path) { return Some(candidate.clone()); } @@ -136,8 +141,20 @@ fn command_basename(command: &str) -> String { .to_string() } -fn canonicalize_if_exists(path: &AbsolutePathBuf) -> AbsolutePathBuf { - path.canonicalize().unwrap_or_else(|_| path.clone()) +async fn canonicalize_if_exists(path: &EnvironmentPathRef) -> EnvironmentPathRef { + path.canonicalize(/*sandbox*/ None) + .await + .unwrap_or_else(|_| path.clone()) +} + +async fn resolve_path_ref(workdir: &EnvironmentPathRef, path: &Path) -> Option { + if path.is_absolute() { + AbsolutePathBuf::from_absolute_path(path) + .ok() + .map(|path| workdir.with_path(path)) + } else { + workdir.join(path).await.ok() + } } #[cfg(test)] diff --git a/codex-rs/core-skills/src/manager.rs b/codex-rs/core-skills/src/manager.rs index 0e5856d00f..1d96e1b203 100644 --- a/codex-rs/core-skills/src/manager.rs +++ b/codex-rs/core-skills/src/manager.rs @@ -190,7 +190,7 @@ impl SkillsManager { self.restriction_product, ); let disabled_paths = resolve_disabled_skill_paths(&outcome.skills, skill_config_rules); - finalize_skill_outcome(outcome, disabled_paths) + finalize_skill_outcome(outcome, disabled_paths).await } pub fn clear_cache(&self) { @@ -294,13 +294,13 @@ fn config_skills_cache_key( } } -fn finalize_skill_outcome( +async fn finalize_skill_outcome( mut outcome: SkillLoadOutcome, disabled_paths: HashSet, ) -> SkillLoadOutcome { outcome.disabled_paths = disabled_paths; let (by_scripts_dir, by_doc_path) = - build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation()); + build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation()).await; outcome.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir); outcome.implicit_skills_by_doc_path = Arc::new(by_doc_path); outcome diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index d5e950e16d..ce8c6f1e38 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -93,8 +93,8 @@ pub struct SkillLoadOutcome { pub disabled_paths: HashSet, pub(crate) skill_roots: Vec, pub(crate) skill_root_by_path: Arc>, - pub(crate) implicit_skills_by_scripts_dir: Arc>, - pub(crate) implicit_skills_by_doc_path: Arc>, + pub(crate) implicit_skills_by_scripts_dir: Arc>, + pub(crate) implicit_skills_by_doc_path: Arc>, } impl SkillLoadOutcome { diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 40d6d50050..4afe5adefd 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -534,7 +534,6 @@ async fn build_skills_and_plugins( warnings: skill_warnings, } = build_skill_injections( &mentioned_skills, - Some(skills_outcome), Some(&turn_context.session_telemetry), &sess.services.analytics_events_client, tracking.clone(), diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 3bdd2b7683..0b425ac3db 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -2,6 +2,7 @@ use super::*; use crate::SkillLoadOutcome; use crate::config::GhostSnapshotConfig; use crate::environment_selection::ResolvedTurnEnvironments; +use codex_exec_server::EnvironmentPathRef; use codex_model_provider::SharedModelProvider; use codex_model_provider::create_model_provider; use codex_protocol::SessionId; @@ -19,7 +20,7 @@ use std::sync::atomic::Ordering; #[derive(Clone, Debug)] pub(crate) struct TurnSkillsContext { pub(crate) outcome: Arc, - pub(crate) implicit_invocation_seen_skills: Arc>>, + pub(crate) implicit_invocation_seen_skills: Arc>>, } impl TurnSkillsContext { diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index d540cd6d43..8ca925782c 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -4,8 +4,7 @@ use crate::session::turn_context::TurnContext; use codex_analytics::InvocationType; use codex_analytics::SkillInvocation; use codex_analytics::build_track_events_context; -use codex_protocol::protocol::SkillScope; -use codex_utils_absolute_path::AbsolutePathBuf; +use codex_exec_server::EnvironmentPathRef; use codex_utils_plugins::PluginSkillRoot; pub use codex_core_skills::SkillError; @@ -49,15 +48,18 @@ pub(crate) async fn maybe_emit_implicit_skill_invocation( sess: &Session, turn_context: &TurnContext, command: &str, - workdir: &AbsolutePathBuf, + workdir: &EnvironmentPathRef, ) { let Some(candidate) = detect_implicit_skill_invocation_for_command( turn_context.turn_skills.outcome.as_ref(), command, workdir, - ) else { + ) + .await + else { return; }; + let source_path = candidate.source_path.clone(); let invocation = SkillInvocation { skill_name: candidate.name, skill_scope: candidate.scope, @@ -65,22 +67,14 @@ pub(crate) async fn maybe_emit_implicit_skill_invocation( plugin_id: candidate.plugin_id, invocation_type: InvocationType::Implicit, }; - let skill_scope = match invocation.skill_scope { - SkillScope::User => "user", - SkillScope::Repo => "repo", - SkillScope::System => "system", - SkillScope::Admin => "admin", - }; - let skill_path = invocation.skill_path.to_string_lossy(); let skill_name = invocation.skill_name.clone(); - let seen_key = format!("{skill_scope}:{skill_path}:{skill_name}"); let inserted = { let mut seen_skills = turn_context .turn_skills .implicit_invocation_seen_skills .lock() .await; - seen_skills.insert(seen_key) + seen_skills.insert(source_path) }; if !inserted { return; diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index 8a501c711c..f35c8b98a0 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -1,3 +1,4 @@ +use codex_exec_server::EnvironmentPathRef; use codex_protocol::ThreadId; use codex_protocol::models::ShellCommandToolCallParams; use codex_tools::ShellCommandBackendConfig; @@ -167,6 +168,11 @@ impl ToolExecutor for ShellCommandHandler { let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?; #[allow(deprecated)] let workdir = turn.resolve_path(params.workdir.clone()); + let workdir = turn + .environments + .primary_filesystem() + .map(|file_system| EnvironmentPathRef::new(file_system, workdir.clone())) + .unwrap_or_else(|| EnvironmentPathRef::local(workdir.clone())); maybe_emit_implicit_skill_invocation( session.as_ref(), turn.as_ref(), diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index 83236881f3..d8eebb3d8c 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -25,6 +25,7 @@ use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcessManager; use crate::unified_exec::generate_chunk_id; +use codex_exec_server::EnvironmentPathRef; use codex_features::Feature; use codex_otel::SessionTelemetry; use codex_otel::TOOL_CALL_UNIFIED_EXEC_METRIC; @@ -136,7 +137,7 @@ impl ToolExecutor for ExecCommandHandler { session.as_ref(), context.turn.as_ref(), &hook_command, - &cwd, + &EnvironmentPathRef::new(Arc::clone(&fs), cwd.clone()), ) .await; let process_id = manager.allocate_process_id().await;