mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
core-skills: route reads through source paths
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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<SkillMetadata>,
|
||||
) -> (
|
||||
HashMap<AbsolutePathBuf, SkillMetadata>,
|
||||
HashMap<AbsolutePathBuf, SkillMetadata>,
|
||||
HashMap<EnvironmentPathRef, SkillMetadata>,
|
||||
HashMap<EnvironmentPathRef, SkillMetadata>,
|
||||
) {
|
||||
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<SkillMetadata> {
|
||||
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<String> {
|
||||
@@ -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<SkillMetadata> {
|
||||
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<SkillMetadata> {
|
||||
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<EnvironmentPathRef> {
|
||||
if path.is_absolute() {
|
||||
AbsolutePathBuf::from_absolute_path(path)
|
||||
.ok()
|
||||
.map(|path| workdir.with_path(path))
|
||||
} else {
|
||||
workdir.join(path).await.ok()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -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<EnvironmentPathRef>,
|
||||
) -> 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
|
||||
|
||||
@@ -93,8 +93,8 @@ pub struct SkillLoadOutcome {
|
||||
pub disabled_paths: HashSet<EnvironmentPathRef>,
|
||||
pub(crate) skill_roots: Vec<EnvironmentPathRef>,
|
||||
pub(crate) skill_root_by_path: Arc<HashMap<EnvironmentPathRef, EnvironmentPathRef>>,
|
||||
pub(crate) implicit_skills_by_scripts_dir: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
|
||||
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
|
||||
pub(crate) implicit_skills_by_scripts_dir: Arc<HashMap<EnvironmentPathRef, SkillMetadata>>,
|
||||
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<EnvironmentPathRef, SkillMetadata>>,
|
||||
}
|
||||
|
||||
impl SkillLoadOutcome {
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<SkillLoadOutcome>,
|
||||
pub(crate) implicit_invocation_seen_skills: Arc<Mutex<HashSet<String>>>,
|
||||
pub(crate) implicit_invocation_seen_skills: Arc<Mutex<HashSet<EnvironmentPathRef>>>,
|
||||
}
|
||||
|
||||
impl TurnSkillsContext {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<ToolInvocation> 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(),
|
||||
|
||||
@@ -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<ToolInvocation> 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;
|
||||
|
||||
Reference in New Issue
Block a user