mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Align implicit skill reads with parser (#27926)
## Summary - reuse the shared shell read parser for implicit skill doc invocation detection - add regression coverage for `nl -ba .../SKILL.md` ## Why Desktop could render `Read User Context skill` for reads recognized by the shared command parser, while implicit `skill_invocation` analytics used a separate reader allowlist and missed cases such as `nl`. ## Validation - `HOME=/private/tmp/codex-core-skills-home-pr PATH=/Users/alexsong/.cache/cargo-home/bin:$PATH CARGO_HOME=/Users/alexsong/.cache/cargo-home just test -p codex-core-skills` - `git diff --cached --check` - `just fmt` attempted; Rust formatting completed, but the Python formatters could not download uncached Ruff wheels because `files.pythonhosted.org` is blocked in this sandbox. - `bazel mod deps --lockfile_mode=update/error --repo_env=ASPECT_TOOLS_TELEMETRY= --repo_env=DO_NOT_TRACK=1` evaluated the module graph and produced no `MODULE.bazel.lock` diff, but Bazel crashed on sandboxed `sysctl` during exit.
This commit is contained in:
committed by
GitHub
Unverified
parent
2ee854eaa5
commit
a57087a865
Generated
+1
@@ -2782,6 +2782,7 @@ dependencies = [
|
||||
"codex-model-provider",
|
||||
"codex-otel",
|
||||
"codex-protocol",
|
||||
"codex-shell-command",
|
||||
"codex-skills",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-output-truncation",
|
||||
|
||||
@@ -23,6 +23,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 }
|
||||
|
||||
@@ -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,33 +103,18 @@ fn detect_skill_doc_read(
|
||||
tokens: &[String],
|
||||
workdir: &AbsolutePathBuf,
|
||||
) -> Option<SkillMetadata> {
|
||||
if !command_reads_file(tokens) {
|
||||
return None;
|
||||
}
|
||||
|
||||
for token in tokens.iter().skip(1) {
|
||||
if token.starts_with('-') {
|
||||
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());
|
||||
for command in parse_command_impl(tokens) {
|
||||
if let ParsedCommand::Read { path, .. } = command {
|
||||
let candidate_path = canonicalize_if_exists(&workdir.join(path.as_path()));
|
||||
if let Some(candidate) = outcome.implicit_skills_by_doc_path.get(&candidate_path) {
|
||||
return Some(candidate.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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;
|
||||
};
|
||||
let program = command_basename(program).to_ascii_lowercase();
|
||||
READERS.contains(&program.as_str())
|
||||
}
|
||||
|
||||
fn command_basename(command: &str) -> String {
|
||||
Path::new(command)
|
||||
.file_name()
|
||||
|
||||
@@ -76,6 +76,30 @@ fn skill_doc_read_detection_matches_absolute_path() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skill_doc_read_detection_matches_shared_read_parser() {
|
||||
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(),
|
||||
"-ba".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();
|
||||
|
||||
Reference in New Issue
Block a user