Inject SKILL.md when it's explicitly mentioned. (#7763)

1. Skills load once in core at session start; the cached outcome is
reused across core and surfaced to TUI via SessionConfigured.
2. TUI detects explicit skill selections, and core injects the matching
SKILL.md content into the turn when a selected skill is present.
This commit is contained in:
xl-openai
2025-12-10 13:59:17 -08:00
committed by GitHub
parent eb2e5458cc
commit b36ecb6c32
21 changed files with 584 additions and 88 deletions

View File

@@ -15,7 +15,7 @@
use crate::config::Config;
use crate::features::Feature;
use crate::skills::load_skills;
use crate::skills::SkillMetadata;
use crate::skills::render_skills_section;
use dunce::canonicalize as normalize_path;
use std::path::PathBuf;
@@ -33,17 +33,12 @@ const PROJECT_DOC_SEPARATOR: &str = "\n\n--- project-doc ---\n\n";
/// Combines `Config::instructions` and `AGENTS.md` (if present) into a single
/// string of instructions.
pub(crate) async fn get_user_instructions(config: &Config) -> Option<String> {
pub(crate) async fn get_user_instructions(
config: &Config,
skills: Option<&[SkillMetadata]>,
) -> Option<String> {
let skills_section = if config.features.enabled(Feature::Skills) {
let skills_outcome = load_skills(config);
for err in &skills_outcome.errors {
error!(
"failed to load skill {}: {}",
err.path.display(),
err.message
);
}
render_skills_section(&skills_outcome.skills)
skills.and_then(render_skills_section)
} else {
None
};
@@ -244,6 +239,7 @@ mod tests {
use super::*;
use crate::config::ConfigOverrides;
use crate::config::ConfigToml;
use crate::skills::load_skills;
use std::fs;
use std::path::PathBuf;
use tempfile::TempDir;
@@ -289,7 +285,7 @@ mod tests {
async fn no_doc_file_returns_none() {
let tmp = tempfile::tempdir().expect("tempdir");
let res = get_user_instructions(&make_config(&tmp, 4096, None)).await;
let res = get_user_instructions(&make_config(&tmp, 4096, None), None).await;
assert!(
res.is_none(),
"Expected None when AGENTS.md is absent and no system instructions provided"
@@ -303,7 +299,7 @@ mod tests {
let tmp = tempfile::tempdir().expect("tempdir");
fs::write(tmp.path().join("AGENTS.md"), "hello world").unwrap();
let res = get_user_instructions(&make_config(&tmp, 4096, None))
let res = get_user_instructions(&make_config(&tmp, 4096, None), None)
.await
.expect("doc expected");
@@ -322,7 +318,7 @@ mod tests {
let huge = "A".repeat(LIMIT * 2); // 2 KiB
fs::write(tmp.path().join("AGENTS.md"), &huge).unwrap();
let res = get_user_instructions(&make_config(&tmp, LIMIT, None))
let res = get_user_instructions(&make_config(&tmp, LIMIT, None), None)
.await
.expect("doc expected");
@@ -354,7 +350,9 @@ mod tests {
let mut cfg = make_config(&repo, 4096, None);
cfg.cwd = nested;
let res = get_user_instructions(&cfg).await.expect("doc expected");
let res = get_user_instructions(&cfg, None)
.await
.expect("doc expected");
assert_eq!(res, "root level doc");
}
@@ -364,7 +362,7 @@ mod tests {
let tmp = tempfile::tempdir().expect("tempdir");
fs::write(tmp.path().join("AGENTS.md"), "something").unwrap();
let res = get_user_instructions(&make_config(&tmp, 0, None)).await;
let res = get_user_instructions(&make_config(&tmp, 0, None), None).await;
assert!(
res.is_none(),
"With limit 0 the function should return None"
@@ -380,7 +378,7 @@ mod tests {
const INSTRUCTIONS: &str = "base instructions";
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)))
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)), None)
.await
.expect("should produce a combined instruction string");
@@ -397,7 +395,7 @@ mod tests {
const INSTRUCTIONS: &str = "some instructions";
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS))).await;
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)), None).await;
assert_eq!(res, Some(INSTRUCTIONS.to_string()));
}
@@ -426,7 +424,9 @@ mod tests {
let mut cfg = make_config(&repo, 4096, None);
cfg.cwd = nested;
let res = get_user_instructions(&cfg).await.expect("doc expected");
let res = get_user_instructions(&cfg, None)
.await
.expect("doc expected");
assert_eq!(res, "root doc\n\ncrate doc");
}
@@ -439,7 +439,7 @@ mod tests {
let cfg = make_config(&tmp, 4096, None);
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, None)
.await
.expect("local doc expected");
@@ -461,7 +461,7 @@ mod tests {
let cfg = make_config_with_fallback(&tmp, 4096, None, &["EXAMPLE.md"]);
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, None)
.await
.expect("fallback doc expected");
@@ -477,7 +477,7 @@ mod tests {
let cfg = make_config_with_fallback(&tmp, 4096, None, &["EXAMPLE.md", ".example.md"]);
let res = get_user_instructions(&cfg)
let res = get_user_instructions(&cfg, None)
.await
.expect("AGENTS.md should win");
@@ -506,9 +506,13 @@ mod tests {
"extract from pdfs",
);
let res = get_user_instructions(&cfg)
.await
.expect("instructions expected");
let skills = load_skills(&cfg);
let res = get_user_instructions(
&cfg,
skills.errors.is_empty().then_some(skills.skills.as_slice()),
)
.await
.expect("instructions expected");
let expected_path = dunce::canonicalize(
cfg.codex_home
.join("skills/pdf-processing/SKILL.md")
@@ -529,9 +533,13 @@ mod tests {
let cfg = make_config(&tmp, 4096, None);
create_skill(cfg.codex_home.clone(), "linting", "run clippy");
let res = get_user_instructions(&cfg)
.await
.expect("instructions expected");
let skills = load_skills(&cfg);
let res = get_user_instructions(
&cfg,
skills.errors.is_empty().then_some(skills.skills.as_slice()),
)
.await
.expect("instructions expected");
let expected_path =
dunce::canonicalize(cfg.codex_home.join("skills/linting/SKILL.md").as_path())
.unwrap_or_else(|_| cfg.codex_home.join("skills/linting/SKILL.md"));