diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2c18d3e573..fba61fba0b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2584,6 +2584,7 @@ dependencies = [ "codex-model-provider", "codex-otel", "codex-protocol", + "codex-shell-command", "codex-skills", "codex-utils-absolute-path", "codex-utils-output-truncation", diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index 4324d29dee..1f07e6235a 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -22,6 +22,7 @@ codex-login = { workspace = true } codex-model-provider = { workspace = true } codex-otel = { workspace = true } codex-protocol = { workspace = true } +codex-shell-command = { workspace = true } codex-skills = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-output-truncation = { workspace = true } diff --git a/codex-rs/core-skills/src/invocation_utils.rs b/codex-rs/core-skills/src/invocation_utils.rs index 4c9d0a4119..6fd80cce6f 100644 --- a/codex-rs/core-skills/src/invocation_utils.rs +++ b/codex-rs/core-skills/src/invocation_utils.rs @@ -3,6 +3,8 @@ use std::path::Path; use crate::SkillLoadOutcome; use crate::SkillMetadata; +use codex_protocol::parse_command::ParsedCommand; +use codex_shell_command::parse_command::parse_command_impl; use codex_utils_absolute_path::AbsolutePathBuf; pub(crate) fn build_implicit_skill_path_indexes( @@ -101,15 +103,11 @@ fn detect_skill_doc_read( tokens: &[String], workdir: &AbsolutePathBuf, ) -> Option { - if !command_reads_file(tokens) { - return None; - } - - for token in tokens.iter().skip(1) { - if token.starts_with('-') { + // Keep partially parsed reads from pipelines such as `cat SKILL.md | head`. + for parsed in parse_command_impl(&tokens_with_command_basename(tokens)) { + let ParsedCommand::Read { path, .. } = parsed else { continue; - } - let path = Path::new(token); + }; let candidate_path = canonicalize_if_exists(&workdir.join(path)); if let Some(candidate) = outcome.implicit_skills_by_doc_path.get(&candidate_path) { return Some(candidate.clone()); @@ -119,13 +117,14 @@ fn detect_skill_doc_read( None } -fn command_reads_file(tokens: &[String]) -> bool { - const READERS: [&str; 8] = ["cat", "sed", "head", "tail", "less", "more", "bat", "awk"]; - let Some(program) = tokens.first() else { - return false; +fn tokens_with_command_basename(tokens: &[String]) -> Vec { + let Some((program, rest)) = tokens.split_first() else { + return Vec::new(); }; - let program = command_basename(program).to_ascii_lowercase(); - READERS.contains(&program.as_str()) + let mut normalized = Vec::with_capacity(tokens.len()); + normalized.push(command_basename(program).to_ascii_lowercase()); + normalized.extend_from_slice(rest); + normalized } fn command_basename(command: &str) -> String { diff --git a/codex-rs/core-skills/src/invocation_utils_tests.rs b/codex-rs/core-skills/src/invocation_utils_tests.rs index ab3a3e8dc0..041ee1e6b7 100644 --- a/codex-rs/core-skills/src/invocation_utils_tests.rs +++ b/codex-rs/core-skills/src/invocation_utils_tests.rs @@ -75,6 +75,29 @@ fn skill_doc_read_detection_matches_absolute_path() { ); } +#[test] +fn skill_doc_read_detection_uses_parsed_read_commands() { + let skill_doc_path = test_path_buf("/tmp/skill-test/SKILL.md").abs(); + let normalized_skill_doc_path = canonicalize_if_exists(&skill_doc_path); + let skill = test_skill_metadata(skill_doc_path); + let outcome = SkillLoadOutcome { + implicit_skills_by_scripts_dir: Arc::new(HashMap::new()), + implicit_skills_by_doc_path: Arc::new(HashMap::from([(normalized_skill_doc_path, skill)])), + ..Default::default() + }; + + let tokens = vec![ + "nl".to_string(), + test_path_display("/tmp/skill-test/SKILL.md"), + ]; + let found = detect_skill_doc_read(&outcome, &tokens, &test_path_buf("/tmp").abs()); + + assert_eq!( + found.map(|value| value.name), + Some("test-skill".to_string()) + ); +} + #[test] fn skill_script_run_detection_matches_relative_path_from_skill_root() { let skill_doc_path = test_path_buf("/tmp/skill-test/SKILL.md").abs(); diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index a68af6a1e2..12ddc0c112 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -3,6 +3,8 @@ use anyhow::Result; use codex_core::ThreadManager; +use codex_core::skills::SkillsLoadInput; +use codex_core::skills::detect_implicit_skill_invocation_for_command; use codex_core::thread_store_from_config; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::EnvironmentManager; @@ -149,6 +151,47 @@ async fn user_turn_includes_skill_instructions() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn parsed_skill_doc_reads_detect_loaded_repo_skill() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let mut builder = test_codex().with_workspace_setup(|cwd, fs| async move { + write_repo_skill(cwd, fs, "demo", "demo skill", "# Body").await + }); + let test = builder.build_remote_aware(&server).await?; + let skills_input = SkillsLoadInput::new( + test.config.cwd.clone(), + Vec::new(), + test.config.config_layer_stack.clone(), + test.config.bundled_skills_enabled(), + ); + let outcome = test + .thread_manager + .skills_manager() + .skills_for_config(&skills_input, Some(test.fs())) + .await; + let skill_path = test.config.cwd.join(".agents/skills/demo/SKILL.md"); + let skill_path_str = skill_path.to_string_lossy(); + + let parsed_read = detect_implicit_skill_invocation_for_command( + &outcome, + &format!("nl {skill_path_str}"), + &test.config.cwd, + ) + .map(|skill| skill.name); + let non_read = detect_implicit_skill_invocation_for_command( + &outcome, + &format!("echo {skill_path_str}"), + &test.config.cwd, + ) + .map(|skill| skill.name); + + assert_eq!((parsed_read, non_read), (Some("demo".to_string()), None)); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn list_skills_includes_repo_and_home_skills_remote_aware() -> Result<()> { skip_if_no_network!(Ok(()));