-
Notifications
You must be signed in to change notification settings - Fork 15.7k
feat: experimental support for skills.md #7412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
e8622da
Add runtime skills discovery and validation
tibo-openai f3552dc
Add skills plan diagram
tibo-openai 5be690a
Update skills plan diagram to mermaid
tibo-openai 1705bf8
Add ASCII skills data flow diagram
tibo-openai e14249e
Update skills plan
tibo-openai ca4680e
Normalize Windows paths in skills tests
gverma-openai ac5b4ba
Merge branch 'main' into tibo/skills
gverma-openai f3122f6
fmt and lint fixes
gverma-openai 100a8bb
Add skills documentation (experimental)
gverma-openai 417168a
Delete design doc
gverma-openai 3c215fe
prettier format new markdown file
gverma-openai 2dfb2fb
Normalize path when extracting for Windows
gverma-openai e401362
Reorganize skills into folder
gverma-openai 9f5fef6
Rename constant
gverma-openai 95fc467
fmt fixes
gverma-openai 75e30f5
Add CLI argument to feature gate loading of skills
gverma-openai e5fea7b
Fix import
gverma-openai f94d631
Add CLI arg feature flag to tests
gverma-openai f5c6184
Check whether target is symlink before following it
gverma-openai 926fe0b
Use feature enum natively
gverma-openai 1cee47c
Return error enum when parsing skills
gverma-openai 9d354e5
Update test to match typed error
gverma-openai 2e282f5
Join strings instead of quadruple-match
gverma-openai 334e98c
Add test suite for loading skills
gverma-openai 1a9a11d
Remove unused import
gverma-openai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,9 @@ | |
| //! 3. We do **not** walk past the Git root. | ||
|
|
||
| use crate::config::Config; | ||
| use crate::features::Feature; | ||
| use crate::skills::load_skills; | ||
| use crate::skills::render_skills_section; | ||
| use dunce::canonicalize as normalize_path; | ||
| use std::path::PathBuf; | ||
| use tokio::io::AsyncReadExt; | ||
|
|
@@ -31,18 +34,37 @@ 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> { | ||
| match read_project_docs(config).await { | ||
| Ok(Some(project_doc)) => match &config.user_instructions { | ||
| Some(original_instructions) => Some(format!( | ||
| "{original_instructions}{PROJECT_DOC_SEPARATOR}{project_doc}" | ||
| )), | ||
| None => Some(project_doc), | ||
| }, | ||
| Ok(None) => config.user_instructions.clone(), | ||
| 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) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let project_docs = match read_project_docs(config).await { | ||
| Ok(docs) => docs, | ||
| Err(e) => { | ||
| error!("error trying to find project doc: {e:#}"); | ||
| config.user_instructions.clone() | ||
| return config.user_instructions.clone(); | ||
| } | ||
| }; | ||
|
|
||
| let combined_project_docs = merge_project_docs_with_skills(project_docs, skills_section); | ||
|
|
||
| match (config.user_instructions.clone(), combined_project_docs) { | ||
|
gverma-openai marked this conversation as resolved.
Outdated
|
||
| (Some(instructions), Some(project_doc)) => Some(format!( | ||
| "{instructions}{PROJECT_DOC_SEPARATOR}{project_doc}" | ||
| )), | ||
| (Some(instructions), None) => Some(instructions), | ||
| (None, Some(project_doc)) => Some(project_doc), | ||
| (None, None) => None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -195,12 +217,25 @@ fn candidate_filenames<'a>(config: &'a Config) -> Vec<&'a str> { | |
| names | ||
| } | ||
|
|
||
| fn merge_project_docs_with_skills( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we get rid of this as well? |
||
| project_doc: Option<String>, | ||
| skills_section: Option<String>, | ||
| ) -> Option<String> { | ||
| match (project_doc, skills_section) { | ||
| (Some(doc), Some(skills)) => Some(format!("{doc}\n\n{skills}")), | ||
| (Some(doc), None) => Some(doc), | ||
| (None, Some(skills)) => Some(skills), | ||
| (None, None) => None, | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::config::ConfigOverrides; | ||
| use crate::config::ConfigToml; | ||
| use std::fs; | ||
| use std::path::PathBuf; | ||
| use tempfile::TempDir; | ||
|
|
||
| /// Helper that returns a `Config` pointing at `root` and using `limit` as | ||
|
|
@@ -219,6 +254,7 @@ mod tests { | |
|
|
||
| config.cwd = root.path().to_path_buf(); | ||
| config.project_doc_max_bytes = limit; | ||
| config.features.enable(Feature::Skills); | ||
|
|
||
| config.user_instructions = instructions.map(ToOwned::to_owned); | ||
| config | ||
|
|
@@ -447,4 +483,58 @@ mod tests { | |
| .eq(DEFAULT_PROJECT_DOC_FILENAME) | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn skills_are_appended_to_project_doc() { | ||
|
tibo-openai marked this conversation as resolved.
|
||
| let tmp = tempfile::tempdir().expect("tempdir"); | ||
| fs::write(tmp.path().join("AGENTS.md"), "base doc").unwrap(); | ||
|
|
||
| let cfg = make_config(&tmp, 4096, None); | ||
| create_skill( | ||
| cfg.codex_home.clone(), | ||
| "pdf-processing", | ||
| "extract from pdfs", | ||
| ); | ||
|
|
||
| let res = get_user_instructions(&cfg) | ||
| .await | ||
| .expect("instructions expected"); | ||
| let expected_path = dunce::canonicalize( | ||
| cfg.codex_home | ||
| .join("skills/pdf-processing/SKILL.md") | ||
| .as_path(), | ||
| ) | ||
| .unwrap_or_else(|_| cfg.codex_home.join("skills/pdf-processing/SKILL.md")); | ||
| let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); | ||
| let expected = format!( | ||
| "base doc\n\n## Skills\nThese skills are discovered at startup from ~/.codex/skills; each entry shows name, description, and file path so you can open the source for full instructions. Content is not inlined to keep context lean.\n- pdf-processing: extract from pdfs (file: {expected_path_str})" | ||
| ); | ||
| assert_eq!(res, expected); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn skills_render_without_project_doc() { | ||
| let tmp = tempfile::tempdir().expect("tempdir"); | ||
| 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 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")); | ||
| let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); | ||
| let expected = format!( | ||
| "## Skills\nThese skills are discovered at startup from ~/.codex/skills; each entry shows name, description, and file path so you can open the source for full instructions. Content is not inlined to keep context lean.\n- linting: run clippy (file: {expected_path_str})" | ||
| ); | ||
| assert_eq!(res, expected); | ||
| } | ||
|
|
||
| fn create_skill(codex_home: PathBuf, name: &str, description: &str) { | ||
| let skill_dir = codex_home.join(format!("skills/{name}")); | ||
| fs::create_dir_all(&skill_dir).unwrap(); | ||
| let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n"); | ||
| fs::write(skill_dir.join("SKILL.md"), content).unwrap(); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.