From 00c2503e7e3f3dc54b80fb35f1366db8addd49a9 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Wed, 1 Jul 2026 17:15:32 +0000 Subject: [PATCH 1/3] test: add plugin skill contract checks --- .../skills/running-impacted-tests/SKILL.md | 3 - .../skills/memorize-subject/SKILL.md | 2 +- .../skills/tracedecay-audit-safety/SKILL.md | 2 +- .../skills/tracedecay-check-health/SKILL.md | 2 +- .../tracedecay-clean-dead-code/SKILL.md | 2 +- .../tracedecay-compare-branches/SKILL.md | 2 +- .../skills/tracedecay-curate-memory/SKILL.md | 2 +- .../skills/tracedecay-draft-commit/SKILL.md | 2 +- .../skills/tracedecay-find-impact/SKILL.md | 2 +- .../skills/tracedecay-fix-build/SKILL.md | 2 +- .../tracedecay-map-architecture/SKILL.md | 2 +- .../skills/tracedecay-port-code/SKILL.md | 2 +- .../skills/tracedecay-recall-memory/SKILL.md | 2 +- .../skills/tracedecay-review-diff/SKILL.md | 2 +- .../skills/tracedecay-test-changes/SKILL.md | 2 +- src/agents/codex.rs | 6 +- tests/codex_plugin_skill_contract_test.rs | 493 ++++++++++++++++++ 17 files changed, 512 insertions(+), 18 deletions(-) create mode 100644 tests/codex_plugin_skill_contract_test.rs diff --git a/codex-plugin/skills/running-impacted-tests/SKILL.md b/codex-plugin/skills/running-impacted-tests/SKILL.md index 09f8503d..f79b24ec 100644 --- a/codex-plugin/skills/running-impacted-tests/SKILL.md +++ b/codex-plugin/skills/running-impacted-tests/SKILL.md @@ -1,9 +1,6 @@ --- name: running-impacted-tests description: 'Use when running only tests affected by changed Rust files, mapping failures back to source, verifying a change without a full suite, or handling cargo-backed impacted-test checks.' -paths: - - "**/*.rs" - - "**/Cargo.toml" --- # Running impacted tests diff --git a/cursor-plugin/skills/memorize-subject/SKILL.md b/cursor-plugin/skills/memorize-subject/SKILL.md index 6e7b3d9b..6d06ec0f 100644 --- a/cursor-plugin/skills/memorize-subject/SKILL.md +++ b/cursor-plugin/skills/memorize-subject/SKILL.md @@ -1,6 +1,6 @@ --- name: memorize-subject -description: 'Research a subject with parallel read-only agents, then store durable facts in TraceDecay memory.' +description: 'Use to research a subject with parallel read-only agents, then store durable facts in TraceDecay memory.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-audit-safety/SKILL.md b/cursor-plugin/skills/tracedecay-audit-safety/SKILL.md index da3baeda..04d5680e 100644 --- a/cursor-plugin/skills/tracedecay-audit-safety/SKILL.md +++ b/cursor-plugin/skills/tracedecay-audit-safety/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-audit-safety -description: 'Audit the repo or a directory for ship-blocking risk — panic sites, risk markers, dead code, and untested high-risk symbols.' +description: 'Use to audit the repo or a directory for ship-blocking risk, panic sites, risk markers, dead code, and untested high-risk symbols.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-check-health/SKILL.md b/cursor-plugin/skills/tracedecay-check-health/SKILL.md index 264b1beb..01c2313b 100644 --- a/cursor-plugin/skills/tracedecay-check-health/SKILL.md +++ b/cursor-plugin/skills/tracedecay-check-health/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-check-health -description: 'Check code health — a scorecard for the repo or a directory with the worst offenders and a prioritized fix list.' +description: 'Use to check code health for the repo or a directory, including worst offenders and a prioritized fix list.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-clean-dead-code/SKILL.md b/cursor-plugin/skills/tracedecay-clean-dead-code/SKILL.md index 4f28f2bc..0efa3db4 100644 --- a/cursor-plugin/skills/tracedecay-clean-dead-code/SKILL.md +++ b/cursor-plugin/skills/tracedecay-clean-dead-code/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-clean-dead-code -description: 'Find and safely remove dead code, unused imports, and duplication via the TraceDecay code graph.' +description: 'Use to find and safely remove dead code, unused imports, and duplication via the TraceDecay code graph.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-compare-branches/SKILL.md b/cursor-plugin/skills/tracedecay-compare-branches/SKILL.md index 8d3ac5a6..ea9a8e60 100644 --- a/cursor-plugin/skills/tracedecay-compare-branches/SKILL.md +++ b/cursor-plugin/skills/tracedecay-compare-branches/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-compare-branches -description: 'Compare or search another git branch''s code graph without switching your checkout.' +description: 'Use to compare or search another git branch''s code graph without switching your checkout.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-curate-memory/SKILL.md b/cursor-plugin/skills/tracedecay-curate-memory/SKILL.md index 26ee4af6..33e5033f 100644 --- a/cursor-plugin/skills/tracedecay-curate-memory/SKILL.md +++ b/cursor-plugin/skills/tracedecay-curate-memory/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-curate-memory -description: 'Curate, update, delete, or inspect tracedecay memory facts and dashboard curation from an explicit slash workflow.' +description: 'Use to curate, update, delete, or inspect TraceDecay memory facts and dashboard curation from an explicit slash workflow.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-draft-commit/SKILL.md b/cursor-plugin/skills/tracedecay-draft-commit/SKILL.md index e7136336..96c70639 100644 --- a/cursor-plugin/skills/tracedecay-draft-commit/SKILL.md +++ b/cursor-plugin/skills/tracedecay-draft-commit/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-draft-commit -description: 'Draft a commit message, PR description, or changelog from the semantic meaning of the changes (drafts text only — never commits or pushes).' +description: 'Use to draft a commit message, PR description, or changelog from semantic changes; drafts text only and never commits or pushes.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-find-impact/SKILL.md b/cursor-plugin/skills/tracedecay-find-impact/SKILL.md index 86cc3c20..a9c58d60 100644 --- a/cursor-plugin/skills/tracedecay-find-impact/SKILL.md +++ b/cursor-plugin/skills/tracedecay-find-impact/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-find-impact -description: 'Find the blast radius of a change — impacted symbols, files, and the tests to run.' +description: 'Use to find the blast radius of a change, including impacted symbols, files, and the tests to run.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-fix-build/SKILL.md b/cursor-plugin/skills/tracedecay-fix-build/SKILL.md index e3743b64..070477e9 100644 --- a/cursor-plugin/skills/tracedecay-fix-build/SKILL.md +++ b/cursor-plugin/skills/tracedecay-fix-build/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-fix-build -description: 'Fix build and type errors — run or parse compiler diagnostics, map each one to its enclosing symbol with callers, then fix.' +description: 'Use to fix build and type errors by running or parsing diagnostics, mapping them to symbols with callers, then fixing.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-map-architecture/SKILL.md b/cursor-plugin/skills/tracedecay-map-architecture/SKILL.md index d28b578b..9b764e40 100644 --- a/cursor-plugin/skills/tracedecay-map-architecture/SKILL.md +++ b/cursor-plugin/skills/tracedecay-map-architecture/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-map-architecture -description: 'Map the architecture of the repo or a directory — layered module map, dependency hotspots, and structural risks.' +description: 'Use to map repo or directory architecture, including layered modules, dependency hotspots, and structural risks.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-port-code/SKILL.md b/cursor-plugin/skills/tracedecay-port-code/SKILL.md index 2c9ff417..4e98b803 100644 --- a/cursor-plugin/skills/tracedecay-port-code/SKILL.md +++ b/cursor-plugin/skills/tracedecay-port-code/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-port-code -description: 'Port or migrate code between directories in dependency-safe order and track progress.' +description: 'Use to port or migrate code between directories in dependency-safe order and track progress.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-recall-memory/SKILL.md b/cursor-plugin/skills/tracedecay-recall-memory/SKILL.md index 15ecc509..b8e1b6e9 100644 --- a/cursor-plugin/skills/tracedecay-recall-memory/SKILL.md +++ b/cursor-plugin/skills/tracedecay-recall-memory/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-recall-memory -description: 'Recall prior decisions, durable facts, and past session conversations for this project.' +description: 'Use to recall prior decisions, durable facts, and past session conversations for this project.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-review-diff/SKILL.md b/cursor-plugin/skills/tracedecay-review-diff/SKILL.md index 0e9c04c6..4789d4df 100644 --- a/cursor-plugin/skills/tracedecay-review-diff/SKILL.md +++ b/cursor-plugin/skills/tracedecay-review-diff/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-review-diff -description: 'Review the current PR/diff for impact, risk, and quality via the TraceDecay code graph.' +description: 'Use to review the current PR or diff for impact, risk, and quality via the TraceDecay code graph.' disable-model-invocation: true --- diff --git a/cursor-plugin/skills/tracedecay-test-changes/SKILL.md b/cursor-plugin/skills/tracedecay-test-changes/SKILL.md index 3d1608ee..26819730 100644 --- a/cursor-plugin/skills/tracedecay-test-changes/SKILL.md +++ b/cursor-plugin/skills/tracedecay-test-changes/SKILL.md @@ -1,6 +1,6 @@ --- name: tracedecay-test-changes -description: 'Test the current changes — run only the affected tests and map failures back to source.' +description: 'Use to test current changes by running only affected tests and mapping failures back to source.' disable-model-invocation: true --- diff --git a/src/agents/codex.rs b/src/agents/codex.rs index 070a0b71..93f53d44 100644 --- a/src/agents/codex.rs +++ b/src/agents/codex.rs @@ -1609,7 +1609,11 @@ mod tests { // workflow Codex intentionally does not ship. The Codex copy inlines // that flow's guardrails (read-only research, dedupe, cited facts, // secret/PII rejection) instead of pointing at a skill absent here. - const CODEX_SKILL_DIVERGENCES: &[&str] = &["curating-project-memory"]; + // - `running-impacted-tests`: Cursor keeps `paths` frontmatter so its + // host can path-scope the skill, while Codex must omit that key to + // satisfy the Codex skill-creator quick_validate.py schema. + const CODEX_SKILL_DIVERGENCES: &[&str] = + &["curating-project-memory", "running-impacted-tests"]; let root = repo_root(); for &skill in crate::hooks::CURSOR_PLUGIN_SKILLS { let codex_path = root diff --git a/tests/codex_plugin_skill_contract_test.rs b/tests/codex_plugin_skill_contract_test.rs new file mode 100644 index 00000000..c6f8bd49 --- /dev/null +++ b/tests/codex_plugin_skill_contract_test.rs @@ -0,0 +1,493 @@ +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use tempfile::TempDir; +use tracedecay::agents::{expected_tool_perms, get_integration, InstallContext}; + +const CODEX_SKILL_ROOT: &str = "codex-plugin/skills"; +const CURSOR_SKILL_ROOT: &str = "cursor-plugin/skills"; +const MAX_SKILL_MD_LINES: usize = 500; +const MAX_DESCRIPTION_CHARS: usize = 320; +const MAX_DESCRIPTION_WORDS: usize = 45; +const MAX_CODEX_METADATA_CHARS: usize = 6_000; +const MAX_CURSOR_MODEL_INVOKED_METADATA_CHARS: usize = 6_000; +const CODEX_QUICK_VALIDATE_ALLOWED_FRONTMATTER: &[&str] = &[ + "allowed-tools", + "description", + "license", + "metadata", + "name", +]; +const CURSOR_ALLOWED_FRONTMATTER: &[&str] = &[ + "allowed-tools", + "description", + "disable-model-invocation", + "license", + "metadata", + "name", + "paths", +]; + +#[derive(Debug)] +struct SkillDoc { + path: PathBuf, + body: String, + frontmatter: BTreeMap, +} + +#[test] +fn codex_plugin_skills_match_codex_skill_creator_quick_validate_rules() { + let skills = load_skill_docs(CODEX_SKILL_ROOT); + assert!(!skills.is_empty(), "expected bundled Codex skills"); + + for skill in &skills { + assert_codex_quick_validate_equivalent(skill); + } +} + +#[test] +fn generated_codex_plugin_skills_match_codex_skill_creator_quick_validate_rules() { + let home = TempDir::new().expect("temp home"); + let codex = get_integration("codex").expect("codex integration"); + codex + .install(&install_ctx(home.path())) + .expect("install generated Codex plugin bundle"); + + let skills = load_skill_docs_from_root(home.path().join("plugins/tracedecay/skills")); + assert_eq!( + skill_names(&skills), + skill_names(&load_skill_docs(CODEX_SKILL_ROOT)), + "generated Codex plugin bundle must ship the same skills as the source bundle" + ); + for skill in &skills { + assert_codex_quick_validate_equivalent(skill); + } +} + +#[test] +fn cursor_plugin_skills_match_cursor_skill_contract() { + let skills = load_skill_docs(CURSOR_SKILL_ROOT); + assert!(!skills.is_empty(), "expected bundled Cursor skills"); + + for skill in &skills { + assert_cursor_skill_contract(skill); + } +} + +#[test] +fn generated_cursor_plugin_skills_match_cursor_skill_contract() { + let home = TempDir::new().expect("temp home"); + let cursor = get_integration("cursor").expect("cursor integration"); + cursor + .install(&install_ctx(home.path())) + .expect("install generated Cursor plugin bundle"); + + let skills = + load_skill_docs_from_root(home.path().join(".cursor/plugins/local/tracedecay/skills")); + assert_eq!( + skill_names(&skills), + skill_names(&load_skill_docs(CURSOR_SKILL_ROOT)), + "generated Cursor plugin bundle must ship the same skills as the source bundle" + ); + for skill in &skills { + assert_cursor_skill_contract(skill); + } +} + +#[test] +fn produced_plugin_skills_follow_skill_creator_design_advice() { + let codex_skills = load_skill_docs(CODEX_SKILL_ROOT); + let cursor_skills = load_skill_docs(CURSOR_SKILL_ROOT); + + assert_metadata_budget("Codex", &codex_skills, MAX_CODEX_METADATA_CHARS, |_| true); + assert_metadata_budget( + "Cursor model-invoked", + &cursor_skills, + MAX_CURSOR_MODEL_INVOKED_METADATA_CHARS, + |skill| !is_cursor_explicit_invoke_only(skill), + ); + + for skill in codex_skills.iter().chain(cursor_skills.iter()) { + let description = skill + .frontmatter + .get("description") + .expect("description present"); + assert_scalar("description", description, &skill.path); + assert!( + has_trigger_language(description), + "{} description must include trigger language because agents only see metadata before loading the body", + skill.path.display() + ); + assert!( + description.len() <= MAX_DESCRIPTION_CHARS, + "{} description is too long for the shared skills metadata budget", + skill.path.display() + ); + assert!( + description.split_whitespace().count() <= MAX_DESCRIPTION_WORDS, + "{} description has too many words for the shared skills metadata budget", + skill.path.display() + ); + + let line_count = skill.body.lines().count(); + assert!( + line_count <= MAX_SKILL_MD_LINES, + "{} has {line_count} lines; split details into direct references before exceeding {MAX_SKILL_MD_LINES}", + skill.path.display() + ); + assert!( + !skill.body.contains("\n## When to Use") + && !skill.body.contains("\n## When To Use") + && !skill.body.contains("\n## When to use"), + "{} must keep trigger guidance in description metadata, not a body-only When to Use section", + skill.path.display() + ); + + let skill_dir = skill.path.parent().expect("skill path has parent"); + assert_skill_tree_uses_supported_files(skill_dir); + assert_openai_yaml_contract_if_present(skill_dir); + } +} + +fn load_skill_docs(root: &str) -> Vec { + load_skill_docs_from_root(Path::new(env!("CARGO_MANIFEST_DIR")).join(root)) +} + +fn load_skill_docs_from_root(skills_root: PathBuf) -> Vec { + let mut paths = std::fs::read_dir(&skills_root) + .unwrap_or_else(|err| { + panic!( + "failed to read bundled skills at {}: {err}", + skills_root.display() + ) + }) + .map(|entry| entry.expect("read skill dir entry").path()) + .filter(|path| path.is_dir()) + .map(|path| path.join("SKILL.md")) + .collect::>(); + paths.sort(); + + paths + .into_iter() + .map(|path| { + let body = std::fs::read_to_string(&path) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())); + let frontmatter = parse_skill_frontmatter(&path, &body); + SkillDoc { + path, + body, + frontmatter, + } + }) + .collect() +} + +fn install_ctx(home: &Path) -> InstallContext { + InstallContext { + home: home.to_path_buf(), + tracedecay_bin: "/tmp/tracedecay-test-bin".to_string(), + tool_permissions: expected_tool_perms(), + profile: None, + project_root: None, + dashboard: true, + } +} + +fn skill_names(skills: &[SkillDoc]) -> Vec { + skills + .iter() + .map(|skill| { + skill + .path + .parent() + .and_then(Path::file_name) + .and_then(|name| name.to_str()) + .expect("skill path has utf-8 directory name") + .to_string() + }) + .collect() +} + +fn assert_codex_quick_validate_equivalent(skill: &SkillDoc) { + assert_allowed_frontmatter(skill, CODEX_QUICK_VALIDATE_ALLOWED_FRONTMATTER); + assert_required_skill_creator_frontmatter(skill); + let description = skill + .frontmatter + .get("description") + .expect("description present"); + assert!( + !description.contains(['<', '>']), + "{} description cannot contain angle brackets", + skill.path.display() + ); + assert!( + description.len() <= 1024, + "{} description exceeds Codex quick_validate.py's 1024 character limit", + skill.path.display() + ); +} + +fn assert_cursor_skill_contract(skill: &SkillDoc) { + assert_allowed_frontmatter(skill, CURSOR_ALLOWED_FRONTMATTER); + assert_required_skill_creator_frontmatter(skill); + + if let Some(disable_model_invocation) = skill.frontmatter.get("disable-model-invocation") { + assert!( + matches!(disable_model_invocation.as_str(), "true" | "false"), + "{} disable-model-invocation must be a boolean scalar", + skill.path.display() + ); + } + if let Some(paths) = skill.frontmatter.get("paths") { + let path_globs = paths + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .collect::>(); + assert!( + !path_globs.is_empty() + && path_globs + .iter() + .all(|line| line.starts_with("- ") && line.len() > 2), + "{} paths must be a non-empty YAML list of path globs", + skill.path.display() + ); + } +} + +fn assert_required_skill_creator_frontmatter(skill: &SkillDoc) { + let skill_dir = skill.path.parent().expect("skill path has parent"); + let folder_name = skill_dir + .file_name() + .and_then(|name| name.to_str()) + .expect("skill dir should be utf-8"); + + let name = skill + .frontmatter + .get("name") + .unwrap_or_else(|| panic!("{} is missing name", skill.path.display())); + let description = skill + .frontmatter + .get("description") + .unwrap_or_else(|| panic!("{} is missing description", skill.path.display())); + + assert_eq!( + name, + folder_name, + "{} skill name must match its folder", + skill.path.display() + ); + assert!( + is_skill_creator_name(name), + "{} skill name must be hyphen-case lowercase letters, digits, and hyphens", + skill.path.display() + ); + assert!( + !name.starts_with('-') && !name.ends_with('-') && !name.contains("--"), + "{} skill name cannot start/end with hyphen or contain consecutive hyphens", + skill.path.display() + ); + assert!( + name.len() <= 64, + "{} skill name exceeds Codex quick_validate.py's 64 character limit", + skill.path.display() + ); + assert_scalar("description", description, &skill.path); +} + +fn assert_allowed_frontmatter(skill: &SkillDoc, allowed: &[&str]) { + let unexpected = skill + .frontmatter + .keys() + .filter(|key| !allowed.contains(&key.as_str())) + .collect::>(); + assert!( + unexpected.is_empty(), + "{} has unexpected frontmatter keys {unexpected:?}; allowed keys are {allowed:?}", + skill.path.display() + ); +} + +fn parse_skill_frontmatter(path: &Path, body: &str) -> BTreeMap { + let rest = body + .strip_prefix("---\n") + .unwrap_or_else(|| panic!("{} must start with YAML frontmatter", path.display())); + let (frontmatter, _) = rest + .split_once("\n---\n") + .unwrap_or_else(|| panic!("{} must close YAML frontmatter", path.display())); + let mut fields: BTreeMap = BTreeMap::new(); + let mut last_key: Option = None; + for line in frontmatter.lines().filter(|line| !line.trim().is_empty()) { + if line.starts_with(char::is_whitespace) { + let Some(key) = &last_key else { + panic!( + "{} has indented frontmatter line before any key: {line:?}", + path.display() + ); + }; + let value = fields + .get_mut(key) + .expect("last frontmatter key should be present"); + if !value.is_empty() { + value.push('\n'); + } + value.push_str(line); + continue; + } + let (key, raw_value) = line + .split_once(':') + .unwrap_or_else(|| panic!("{} has invalid frontmatter line {line:?}", path.display())); + assert!( + fields + .insert( + key.to_string(), + unquote_frontmatter_scalar(raw_value.trim()) + ) + .is_none(), + "{} duplicates frontmatter key {key}", + path.display() + ); + last_key = Some(key.to_string()); + } + fields +} + +fn unquote_frontmatter_scalar(value: &str) -> String { + if let Some(inner) = value.strip_prefix('\'').and_then(|v| v.strip_suffix('\'')) { + inner.to_string() + } else if let Some(inner) = value.strip_prefix('"').and_then(|v| v.strip_suffix('"')) { + inner.to_string() + } else { + value.to_string() + } +} + +fn assert_metadata_budget( + label: &str, + skills: &[SkillDoc], + max_total_chars: usize, + include: impl Fn(&SkillDoc) -> bool, +) { + let total_metadata_chars = skills + .iter() + .filter(|skill| include(skill)) + .map(|skill| { + skill.frontmatter.get("name").map_or(0, String::len) + + skill.frontmatter.get("description").map_or(0, String::len) + }) + .sum::(); + + assert!( + total_metadata_chars <= max_total_chars, + "{label} skill metadata uses {total_metadata_chars} chars; keep bundled descriptions concise" + ); +} + +fn assert_scalar(field: &str, value: &str, path: &Path) { + assert!( + !value.trim().is_empty(), + "{} frontmatter {field} cannot be empty", + path.display() + ); + assert_eq!( + value.trim(), + value, + "{} frontmatter {field} cannot have leading or trailing whitespace", + path.display() + ); + assert!( + !value.contains(['\n', '\r']), + "{} frontmatter {field} must be a single line", + path.display() + ); +} + +fn is_skill_creator_name(name: &str) -> bool { + !name.is_empty() + && name + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') +} + +fn has_trigger_language(description: &str) -> bool { + description.contains("Use when") + || description.contains("Use before") + || description.contains("Use to") +} + +fn is_cursor_explicit_invoke_only(skill: &SkillDoc) -> bool { + skill + .frontmatter + .get("disable-model-invocation") + .is_some_and(|value| value == "true") +} + +fn assert_skill_tree_uses_supported_files(skill_dir: &Path) { + let allowed_resource_dirs = ["agents", "scripts", "references", "assets"]; + let forbidden_doc_files = [ + "README.md", + "CHANGELOG.md", + "INSTALLATION_GUIDE.md", + "QUICK_REFERENCE.md", + ]; + let mut stack = vec![skill_dir.to_path_buf()]; + while let Some(dir) = stack.pop() { + for entry in std::fs::read_dir(&dir) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", dir.display())) + { + let entry = entry.expect("read skill tree entry"); + let path = entry.path(); + let relative = path.strip_prefix(skill_dir).expect("relative skill path"); + let first = relative + .components() + .next() + .and_then(|component| component.as_os_str().to_str()) + .expect("relative component"); + if path.is_dir() { + assert!( + allowed_resource_dirs.contains(&first), + "{} contains unsupported top-level directory {}", + skill_dir.display(), + first + ); + stack.push(path); + continue; + } + + let file_name = path + .file_name() + .and_then(|name| name.to_str()) + .expect("skill file name should be utf-8"); + assert!( + !forbidden_doc_files.contains(&file_name), + "{} contains auxiliary documentation file {}; keep skill folders lean", + skill_dir.display(), + file_name + ); + assert!( + relative == Path::new("SKILL.md") || allowed_resource_dirs.contains(&first), + "{} contains unsupported top-level file {}", + skill_dir.display(), + relative.display() + ); + } + } +} + +fn assert_openai_yaml_contract_if_present(skill_dir: &Path) { + let openai_yaml = skill_dir.join("agents/openai.yaml"); + if !openai_yaml.exists() { + return; + } + let body = std::fs::read_to_string(&openai_yaml) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", openai_yaml.display())); + for field in ["display_name:", "short_description:", "default_prompt:"] { + assert!( + body.lines().any(|line| line.starts_with(field)), + "{} must include {field}", + openai_yaml.display() + ); + } +} From f346766b5d7b2bdc5e68f1e68923492bbf14df95 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Wed, 1 Jul 2026 22:05:56 +0000 Subject: [PATCH 2/3] refactor: add canonical skill frontmatter parser Give the crate one line-ending-tolerant SKILL.md frontmatter parser with typed Scalar/Block values and proper YAML single-quote unescaping, and rewire the Hermes skill inventory onto it instead of its bespoke copy. --- src/automation/hermes_skill_inventory.rs | 40 ++-- src/automation/mod.rs | 1 + src/automation/skill_frontmatter.rs | 225 ++++++++++++++++++ ..._test.rs => plugin_skill_contract_test.rs} | 0 4 files changed, 244 insertions(+), 22 deletions(-) create mode 100644 src/automation/skill_frontmatter.rs rename tests/{codex_plugin_skill_contract_test.rs => plugin_skill_contract_test.rs} (100%) diff --git a/src/automation/hermes_skill_inventory.rs b/src/automation/hermes_skill_inventory.rs index e8699fcf..3425cc53 100644 --- a/src/automation/hermes_skill_inventory.rs +++ b/src/automation/hermes_skill_inventory.rs @@ -8,6 +8,7 @@ use serde_json::Value; use crate::errors::{Result, TraceDecayError}; use super::hermes_config_projection::{load_hermes_yaml_projection, yaml_bool}; +use super::skill_frontmatter::{parse_skill_frontmatter, SkillFrontmatterValue}; const SKILL_MD: &str = "SKILL.md"; pub(crate) const USAGE_FILE: &str = ".usage.json"; @@ -354,29 +355,24 @@ pub(crate) fn count_archive_entries(archive_dir: &Path) -> Result { Ok(entries.filter_map(std::result::Result::ok).count()) } +/// Lenient view over [`parse_skill_frontmatter`]: hub skills without (or with +/// malformed) frontmatter degrade to an empty map so inventory listing never +/// fails, and only scalar values matter here (`name`/`id`/`description`/ +/// `summary` are all scalars). fn parse_frontmatter(contents: &str) -> BTreeMap { - let mut lines = contents.lines(); - if lines.next() != Some("---") { - return BTreeMap::new(); - } - let mut values = BTreeMap::new(); - for line in lines { - if line.trim() == "---" { - break; - } - let Some((key, value)) = line.split_once(':') else { - continue; - }; - values.insert( - key.trim().to_ascii_lowercase(), - value - .trim() - .trim_matches('"') - .trim_matches('\'') - .to_string(), - ); - } - values + parse_skill_frontmatter(contents) + .map(|fields| { + fields + .into_iter() + .filter_map(|(key, value)| match value { + SkillFrontmatterValue::Scalar(scalar) => { + Some((key.to_ascii_lowercase(), scalar)) + } + SkillFrontmatterValue::Block(_) => None, + }) + .collect() + }) + .unwrap_or_default() } fn truncate_chars(value: &str, max_chars: usize) -> String { diff --git a/src/automation/mod.rs b/src/automation/mod.rs index cf1a034d..4266d3f9 100644 --- a/src/automation/mod.rs +++ b/src/automation/mod.rs @@ -22,6 +22,7 @@ pub mod run_ledger; pub mod runner; pub mod scheduler; pub mod session_reflector; +pub mod skill_frontmatter; pub mod skill_targets; pub mod skill_usage; pub mod skill_writer; diff --git a/src/automation/skill_frontmatter.rs b/src/automation/skill_frontmatter.rs new file mode 100644 index 00000000..04a3eb8c --- /dev/null +++ b/src/automation/skill_frontmatter.rs @@ -0,0 +1,225 @@ +//! Canonical parser for `SKILL.md` YAML frontmatter. +//! +//! Skill frontmatter across the repo (bundled `codex-plugin/` and +//! `cursor-plugin/` skills, Hermes hub skills, agent-managed exports) uses a +//! small YAML subset: a `---` fence, `key: value` scalars (optionally single- +//! or double-quoted), and block values made of indented lines (list items or +//! nested maps). This module is the one place that subset is parsed so +//! consumers ([`crate::automation::hermes_skill_inventory`] and the plugin +//! contract tests in `tests/plugin_skill_contract_test.rs`) stop growing +//! bespoke, subtly different parsers. +//! +//! Parsing is line-ending tolerant: CRLF checkouts (e.g. GitHub Windows +//! runners with `core.autocrlf=true`) parse identically to LF checkouts. + +use std::collections::BTreeMap; + +use crate::errors::{Result, TraceDecayError}; + +/// One frontmatter value: either an inline scalar (`key: value`) or a block +/// of indented continuation lines (`key:` followed by list items or nested +/// mappings). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SkillFrontmatterValue { + /// Inline scalar with quoting already resolved (outer quotes stripped, + /// YAML `''` doubling inside single-quoted scalars unescaped). + Scalar(String), + /// Raw trimmed block lines under a key with no inline value. + Block(Vec), +} + +impl SkillFrontmatterValue { + pub fn as_scalar(&self) -> Option<&str> { + match self { + Self::Scalar(scalar) => Some(scalar), + Self::Block(_) => None, + } + } + + /// Returns the unquoted items of a block whose every line is a YAML list + /// item (`- item`), or `None` for scalars and other block shapes (nested + /// maps, empty blocks). + pub fn as_list_items(&self) -> Option> { + match self { + Self::Scalar(_) => None, + Self::Block(lines) => { + if lines.is_empty() { + return None; + } + lines + .iter() + .map(|line| { + line.strip_prefix("- ") + .map(|item| unquote_scalar(item.trim())) + }) + .collect() + } + } + } +} + +/// Parses the leading `---`-fenced YAML frontmatter of a `SKILL.md` document. +/// +/// Returns an error when the document does not open with frontmatter, never +/// closes it, repeats a key, or contains a top-level line that is not a +/// `key: value` mapping. Line endings (`\n` vs `\r\n`) are normalized away. +pub fn parse_skill_frontmatter(contents: &str) -> Result> { + let mut lines = contents.lines(); + if lines.next().map(str::trim_end) != Some("---") { + return Err(config_error("must start with YAML frontmatter")); + } + + let mut fields: BTreeMap = BTreeMap::new(); + let mut last_key: Option = None; + let mut closed = false; + for line in lines { + if line.trim() == "---" { + closed = true; + break; + } + if line.trim().is_empty() { + continue; + } + if line.starts_with(char::is_whitespace) { + let Some(key) = &last_key else { + return Err(config_error(format!( + "has indented frontmatter line before any key: {line:?}" + ))); + }; + let Some(value) = fields.get_mut(key) else { + return Err(config_error(format!( + "lost track of frontmatter key {key} while parsing a block" + ))); + }; + match value { + SkillFrontmatterValue::Block(block_lines) => { + block_lines.push(line.trim().to_string()); + } + SkillFrontmatterValue::Scalar(_) => { + return Err(config_error(format!( + "key {key} mixes an inline scalar with block continuation lines" + ))); + } + } + continue; + } + let Some((key, raw_value)) = line.split_once(':') else { + return Err(config_error(format!( + "has invalid frontmatter line {line:?}" + ))); + }; + let key = key.trim().to_string(); + let raw_value = raw_value.trim(); + let value = if raw_value.is_empty() { + SkillFrontmatterValue::Block(Vec::new()) + } else { + SkillFrontmatterValue::Scalar(unquote_scalar(raw_value)) + }; + if fields.insert(key.clone(), value).is_some() { + return Err(config_error(format!("duplicates frontmatter key {key}"))); + } + last_key = Some(key); + } + if !closed { + return Err(config_error("must close YAML frontmatter")); + } + Ok(fields) +} + +/// Strips one level of YAML quoting: single-quoted scalars also unescape the +/// `''` doubling YAML uses to embed a literal `'`. Skill frontmatter never +/// uses backslash escapes, so double-quoted scalars only lose their quotes. +fn unquote_scalar(value: &str) -> String { + if let Some(inner) = value.strip_prefix('\'').and_then(|v| v.strip_suffix('\'')) { + inner.replace("''", "'") + } else if let Some(inner) = value.strip_prefix('"').and_then(|v| v.strip_suffix('"')) { + inner.to_string() + } else { + value.to_string() + } +} + +fn config_error(message: impl Into) -> TraceDecayError { + TraceDecayError::Config { + message: message.into(), + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::{parse_skill_frontmatter, SkillFrontmatterValue}; + + #[test] + fn parses_scalars_blocks_and_quoting() { + let doc = concat!( + "---\n", + "name: my-skill\n", + "description: 'Use when a branch''s graph is compared.'\n", + "paths:\n", + " - \"**/*.rs\"\n", + " - \"**/Cargo.toml\"\n", + "---\n", + "\n# Body\n" + ); + let fields = parse_skill_frontmatter(doc).unwrap(); + assert_eq!(fields["name"].as_scalar(), Some("my-skill")); + assert_eq!( + fields["description"].as_scalar(), + Some("Use when a branch's graph is compared."), + "single-quote doubling must be unescaped" + ); + assert_eq!( + fields["paths"].as_list_items(), + Some(vec!["**/*.rs".to_string(), "**/Cargo.toml".to_string()]) + ); + } + + /// GitHub Windows runners check out with `core.autocrlf=true`, so every + /// SKILL.md arrives with CRLF line endings; parsing must not depend on + /// exact `---\n` byte sequences. + #[test] + fn parses_crlf_documents_identically_to_lf() { + let lf = "---\nname: my-skill\ndescription: Use when testing.\npaths:\n - \"**/*.rs\"\n---\n\n# Body\n"; + let crlf = lf.replace('\n', "\r\n"); + assert_eq!( + parse_skill_frontmatter(&crlf).unwrap(), + parse_skill_frontmatter(lf).unwrap() + ); + } + + #[test] + fn nested_maps_are_blocks_but_not_list_items() { + let doc = "---\nname: my-skill\nmetadata:\n author: someone\n---\nBody\n"; + let fields = parse_skill_frontmatter(doc).unwrap(); + assert_eq!( + fields["metadata"], + SkillFrontmatterValue::Block(vec!["author: someone".to_string()]) + ); + assert_eq!(fields["metadata"].as_list_items(), None); + assert_eq!(fields["metadata"].as_scalar(), None); + } + + #[test] + fn rejects_malformed_frontmatter() { + for (doc, reason) in [ + ("# no frontmatter\n", "missing opening fence"), + ("---\nname: x\n", "unclosed frontmatter"), + ("---\nname: x\nname: y\n---\n", "duplicate key"), + ("---\njust some text\n---\n", "non-mapping line"), + ( + "---\n - orphan\nname: x\n---\n", + "indented line before any key", + ), + ( + "---\nname: x\n - continuation\n---\n", + "block continuation under an inline scalar", + ), + ] { + assert!( + parse_skill_frontmatter(doc).is_err(), + "expected parse error for {reason}: {doc:?}" + ); + } + } +} diff --git a/tests/codex_plugin_skill_contract_test.rs b/tests/plugin_skill_contract_test.rs similarity index 100% rename from tests/codex_plugin_skill_contract_test.rs rename to tests/plugin_skill_contract_test.rs From bc2129f08d844badc632810e7870bb8776542966 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Wed, 1 Jul 2026 22:05:56 +0000 Subject: [PATCH 3/3] test: address plugin skill contract review findings Reuse the canonical frontmatter parser so the contract tests survive CRLF checkouts (this PR's Windows CI failure), replace the generated-bundle contract re-runs with byte-parity against the source tree, pin TRACEDECAY_DATA_DIR for hermetic installs, narrow the running-impacted-tests parity exemption to frontmatter only, loosen the trigger-language heuristic to a documented rule, document the size budgets, and rename the test to plugin_skill_contract_test.rs to match its Codex+Cursor scope. --- src/agents/codex.rs | 44 +++- tests/plugin_skill_contract_test.rs | 373 ++++++++++++++-------------- 2 files changed, 224 insertions(+), 193 deletions(-) diff --git a/src/agents/codex.rs b/src/agents/codex.rs index 93f53d44..f170184a 100644 --- a/src/agents/codex.rs +++ b/src/agents/codex.rs @@ -1597,11 +1597,13 @@ mod tests { /// frontmatter for invocation and ignores extra keys, and the skill bodies /// reference host-neutral `tracedecay_*` MCP tools, so the same content is /// correct in both hosts. Intentional per-skill divergences must be listed - /// (with a reason) in `CODEX_SKILL_DIVERGENCES`. + /// (with a reason) in one of the divergence allowlists below. Per-host + /// frontmatter schemas (allowed keys per plugin) are enforced separately + /// by `tests/plugin_skill_contract_test.rs`. #[test] fn codex_skills_match_the_cursor_source_for_parity() { // Skills deliberately specialized for Codex (host-specific bodies that - // are not byte-compared against the Cursor source): + // are not compared against the Cursor source at all): // // - `curating-project-memory`: the Cursor source hands the "add a // researched subject from scratch" flow off to the `memorizing-subject` @@ -1609,11 +1611,14 @@ mod tests { // workflow Codex intentionally does not ship. The Codex copy inlines // that flow's guardrails (read-only research, dedupe, cited facts, // secret/PII rejection) instead of pointing at a skill absent here. + const CODEX_SKILL_BODY_DIVERGENCES: &[&str] = &["curating-project-memory"]; + // Skills whose frontmatter legitimately diverges while the bodies must + // still mirror byte-for-byte (compared after stripping frontmatter): + // // - `running-impacted-tests`: Cursor keeps `paths` frontmatter so its // host can path-scope the skill, while Codex must omit that key to // satisfy the Codex skill-creator quick_validate.py schema. - const CODEX_SKILL_DIVERGENCES: &[&str] = - &["curating-project-memory", "running-impacted-tests"]; + const CODEX_SKILL_FRONTMATTER_DIVERGENCES: &[&str] = &["running-impacted-tests"]; let root = repo_root(); for &skill in crate::hooks::CURSOR_PLUGIN_SKILLS { let codex_path = root @@ -1624,7 +1629,7 @@ mod tests { codex_path.exists(), "Codex plugin must ship the `{skill}` skill for parity with Cursor" ); - if CODEX_SKILL_DIVERGENCES.contains(&skill) { + if CODEX_SKILL_BODY_DIVERGENCES.contains(&skill) { continue; } let cursor_body = std::fs::read_to_string( @@ -1635,14 +1640,41 @@ mod tests { .expect("cursor skill source should be readable"); let codex_body = std::fs::read_to_string(&codex_path) .expect("codex skill source should be readable"); + if CODEX_SKILL_FRONTMATTER_DIVERGENCES.contains(&skill) { + assert_eq!( + lines_after_frontmatter(&codex_body), + lines_after_frontmatter(&cursor_body), + "Codex `{skill}` skill body must mirror the Cursor source even though \ + its frontmatter intentionally diverges" + ); + continue; + } assert_eq!( codex_body, cursor_body, "Codex `{skill}` skill must mirror the Cursor source (add it to \ - CODEX_SKILL_DIVERGENCES if a host-specific version is intended)" + a CODEX_SKILL_*_DIVERGENCES list if a host-specific version is intended)" ); } } + /// Returns the lines following the closing `---` of the leading YAML + /// frontmatter. Line-based so CRLF checkouts compare like LF ones. + fn lines_after_frontmatter(contents: &str) -> Vec<&str> { + let mut lines = contents.lines(); + assert_eq!( + lines.next(), + Some("---"), + "skill must open YAML frontmatter" + ); + let mut lines = lines.skip_while(|line| line.trim() != "---"); + assert_eq!( + lines.next().map(str::trim), + Some("---"), + "skill must close YAML frontmatter" + ); + lines.collect() + } + /// Extracts the `` from every `tracedecay:` skill handoff in a /// body. MCP tool calls use `tracedecay_*` (underscore) and are ignored. fn skill_handoff_references(body: &str) -> Vec { diff --git a/tests/plugin_skill_contract_test.rs b/tests/plugin_skill_contract_test.rs index c6f8bd49..a65c28d9 100644 --- a/tests/plugin_skill_contract_test.rs +++ b/tests/plugin_skill_contract_test.rs @@ -1,18 +1,38 @@ +//! Contract tests for the bundled Codex (`codex-plugin/`) and Cursor +//! (`cursor-plugin/`) plugin skills: frontmatter schema per host, plus the +//! shared skill-creator design-advice checks. +//! +//! Cross-host parity — each Codex skill mirroring its Cursor source, and the +//! divergence allowlists — is covered by the unit tests in +//! `src/agents/codex.rs` (`codex_skills_match_the_cursor_source_for_parity`). + #![allow(clippy::unwrap_used, clippy::expect_used)] +mod common; + use std::collections::BTreeMap; use std::path::{Path, PathBuf}; +use std::sync::Mutex; +use common::EnvVarGuard; use tempfile::TempDir; use tracedecay::agents::{expected_tool_perms, get_integration, InstallContext}; +use tracedecay::automation::skill_frontmatter::{parse_skill_frontmatter, SkillFrontmatterValue}; +use tracedecay::config::USER_DATA_DIR_ENV; const CODEX_SKILL_ROOT: &str = "codex-plugin/skills"; const CURSOR_SKILL_ROOT: &str = "cursor-plugin/skills"; +// Size budgets: the 500-line body cap and the "concise, trigger-first +// description" rule come from Anthropic's skill-creator design advice. The +// numeric description and metadata caps are house budgets chosen when these +// bundles were written: one description stays scannable at roughly two +// sentences (320 chars / 45 words), and a bundle's preloaded name+description +// metadata stays under 6,000 chars (~1.5k tokens) so skill discovery never +// crowds an agent host's context window. const MAX_SKILL_MD_LINES: usize = 500; const MAX_DESCRIPTION_CHARS: usize = 320; const MAX_DESCRIPTION_WORDS: usize = 45; -const MAX_CODEX_METADATA_CHARS: usize = 6_000; -const MAX_CURSOR_MODEL_INVOKED_METADATA_CHARS: usize = 6_000; +const MAX_BUNDLED_SKILL_METADATA_CHARS: usize = 6_000; const CODEX_QUICK_VALIDATE_ALLOWED_FRONTMATTER: &[&str] = &[ "allowed-tools", "description", @@ -30,11 +50,14 @@ const CURSOR_ALLOWED_FRONTMATTER: &[&str] = &[ "paths", ]; +/// Serializes the generated-bundle tests, which mutate process-wide env vars. +static INSTALL_ENV_LOCK: Mutex<()> = Mutex::new(()); + #[derive(Debug)] struct SkillDoc { path: PathBuf, body: String, - frontmatter: BTreeMap, + frontmatter: BTreeMap, } #[test] @@ -48,22 +71,23 @@ fn codex_plugin_skills_match_codex_skill_creator_quick_validate_rules() { } #[test] -fn generated_codex_plugin_skills_match_codex_skill_creator_quick_validate_rules() { +fn generated_codex_plugin_skills_are_byte_copies_of_the_source_bundle() { + let _env_lock = install_env_lock(); let home = TempDir::new().expect("temp home"); + let _data_dir_guard = pinned_profile_storage(home.path()); let codex = get_integration("codex").expect("codex integration"); codex .install(&install_ctx(home.path())) .expect("install generated Codex plugin bundle"); - let skills = load_skill_docs_from_root(home.path().join("plugins/tracedecay/skills")); + let source_root = skills_source_root(CODEX_SKILL_ROOT); + let installed_root = home.path().join("plugins/tracedecay/skills"); assert_eq!( - skill_names(&skills), - skill_names(&load_skill_docs(CODEX_SKILL_ROOT)), + skill_dir_names(&installed_root), + skill_dir_names(&source_root), "generated Codex plugin bundle must ship the same skills as the source bundle" ); - for skill in &skills { - assert_codex_quick_validate_equivalent(skill); - } + assert_skill_trees_byte_identical(&source_root, &installed_root); } #[test] @@ -77,23 +101,23 @@ fn cursor_plugin_skills_match_cursor_skill_contract() { } #[test] -fn generated_cursor_plugin_skills_match_cursor_skill_contract() { +fn generated_cursor_plugin_skills_are_byte_copies_of_the_source_bundle() { + let _env_lock = install_env_lock(); let home = TempDir::new().expect("temp home"); + let _data_dir_guard = pinned_profile_storage(home.path()); let cursor = get_integration("cursor").expect("cursor integration"); cursor .install(&install_ctx(home.path())) .expect("install generated Cursor plugin bundle"); - let skills = - load_skill_docs_from_root(home.path().join(".cursor/plugins/local/tracedecay/skills")); + let source_root = skills_source_root(CURSOR_SKILL_ROOT); + let installed_root = home.path().join(".cursor/plugins/local/tracedecay/skills"); assert_eq!( - skill_names(&skills), - skill_names(&load_skill_docs(CURSOR_SKILL_ROOT)), + skill_dir_names(&installed_root), + skill_dir_names(&source_root), "generated Cursor plugin bundle must ship the same skills as the source bundle" ); - for skill in &skills { - assert_cursor_skill_contract(skill); - } + assert_skill_trees_byte_identical(&source_root, &installed_root); } #[test] @@ -101,20 +125,13 @@ fn produced_plugin_skills_follow_skill_creator_design_advice() { let codex_skills = load_skill_docs(CODEX_SKILL_ROOT); let cursor_skills = load_skill_docs(CURSOR_SKILL_ROOT); - assert_metadata_budget("Codex", &codex_skills, MAX_CODEX_METADATA_CHARS, |_| true); - assert_metadata_budget( - "Cursor model-invoked", - &cursor_skills, - MAX_CURSOR_MODEL_INVOKED_METADATA_CHARS, - |skill| !is_cursor_explicit_invoke_only(skill), - ); + assert_metadata_budget("Codex", &codex_skills, |_| true); + assert_metadata_budget("Cursor model-invoked", &cursor_skills, |skill| { + !is_cursor_explicit_invoke_only(skill) + }); for skill in codex_skills.iter().chain(cursor_skills.iter()) { - let description = skill - .frontmatter - .get("description") - .expect("description present"); - assert_scalar("description", description, &skill.path); + let description = required_scalar_field(skill, "description"); assert!( has_trigger_language(description), "{} description must include trigger language because agents only see metadata before loading the body", @@ -138,9 +155,7 @@ fn produced_plugin_skills_follow_skill_creator_design_advice() { skill.path.display() ); assert!( - !skill.body.contains("\n## When to Use") - && !skill.body.contains("\n## When To Use") - && !skill.body.contains("\n## When to use"), + !skill.body.to_ascii_lowercase().contains("\n## when to use"), "{} must keep trigger guidance in description metadata, not a body-only When to Use section", skill.path.display() ); @@ -151,11 +166,12 @@ fn produced_plugin_skills_follow_skill_creator_design_advice() { } } -fn load_skill_docs(root: &str) -> Vec { - load_skill_docs_from_root(Path::new(env!("CARGO_MANIFEST_DIR")).join(root)) +fn skills_source_root(root: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join(root) } -fn load_skill_docs_from_root(skills_root: PathBuf) -> Vec { +fn load_skill_docs(root: &str) -> Vec { + let skills_root = skills_source_root(root); let mut paths = std::fs::read_dir(&skills_root) .unwrap_or_else(|err| { panic!( @@ -174,7 +190,8 @@ fn load_skill_docs_from_root(skills_root: PathBuf) -> Vec { .map(|path| { let body = std::fs::read_to_string(&path) .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())); - let frontmatter = parse_skill_frontmatter(&path, &body); + let frontmatter = parse_skill_frontmatter(&body) + .unwrap_or_else(|err| panic!("{}: {err}", path.display())); SkillDoc { path, body, @@ -184,6 +201,19 @@ fn load_skill_docs_from_root(skills_root: PathBuf) -> Vec { .collect() } +fn install_env_lock() -> std::sync::MutexGuard<'static, ()> { + INSTALL_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + +/// Pins TraceDecay profile storage to the temp home so an ambient +/// `TRACEDECAY_DATA_DIR` with active managed skills cannot leak an +/// `agent-managed` overlay into the generated bundle. +fn pinned_profile_storage(home: &Path) -> EnvVarGuard { + EnvVarGuard::set(USER_DATA_DIR_ENV, home.join(".tracedecay")) +} + fn install_ctx(home: &Path) -> InstallContext { InstallContext { home: home.to_path_buf(), @@ -195,28 +225,77 @@ fn install_ctx(home: &Path) -> InstallContext { } } -fn skill_names(skills: &[SkillDoc]) -> Vec { - skills - .iter() - .map(|skill| { - skill - .path - .parent() - .and_then(Path::file_name) +fn skill_dir_names(skills_root: &Path) -> Vec { + let mut names = std::fs::read_dir(skills_root) + .unwrap_or_else(|err| panic!("failed to read skills at {}: {err}", skills_root.display())) + .map(|entry| entry.expect("read skill dir entry").path()) + .filter(|path| path.is_dir()) + .map(|path| { + path.file_name() .and_then(|name| name.to_str()) - .expect("skill path has utf-8 directory name") + .expect("skill directory name should be utf-8") .to_string() }) - .collect() + .collect::>(); + names.sort(); + names +} + +/// The installed skills are `include_str!` byte-copies of the source tree +/// (`src/agents/codex.rs` asserts the embedded list covers every source +/// file), so byte-parity subsumes re-running the per-skill contract over the +/// installed copies and additionally catches any install-time mutation. +fn assert_skill_trees_byte_identical(source_root: &Path, installed_root: &Path) { + let source_files = relative_files_under(source_root); + let installed_files = relative_files_under(installed_root); + assert_eq!( + installed_files, + source_files, + "installed skill tree {} must contain exactly the files of source tree {}", + installed_root.display(), + source_root.display() + ); + for relative in &source_files { + let read = |root: &Path| { + let path = root.join(relative); + std::fs::read(&path) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", path.display())) + }; + assert!( + read(installed_root) == read(source_root), + "installed {} must be a byte-identical copy of the source skill file", + installed_root.join(relative).display() + ); + } +} + +fn relative_files_under(root: &Path) -> Vec { + let mut files = Vec::new(); + let mut stack = vec![root.to_path_buf()]; + while let Some(dir) = stack.pop() { + for entry in std::fs::read_dir(&dir) + .unwrap_or_else(|err| panic!("failed to read {}: {err}", dir.display())) + { + let path = entry.expect("read skill tree entry").path(); + if path.is_dir() { + stack.push(path); + } else { + files.push( + path.strip_prefix(root) + .expect("collected paths live under root") + .to_path_buf(), + ); + } + } + } + files.sort(); + files } fn assert_codex_quick_validate_equivalent(skill: &SkillDoc) { assert_allowed_frontmatter(skill, CODEX_QUICK_VALIDATE_ALLOWED_FRONTMATTER); assert_required_skill_creator_frontmatter(skill); - let description = skill - .frontmatter - .get("description") - .expect("description present"); + let description = required_scalar_field(skill, "description"); assert!( !description.contains(['<', '>']), "{} description cannot contain angle brackets", @@ -233,24 +312,19 @@ fn assert_cursor_skill_contract(skill: &SkillDoc) { assert_allowed_frontmatter(skill, CURSOR_ALLOWED_FRONTMATTER); assert_required_skill_creator_frontmatter(skill); - if let Some(disable_model_invocation) = skill.frontmatter.get("disable-model-invocation") { + if let Some(disable_model_invocation) = scalar_field(skill, "disable-model-invocation") { assert!( - matches!(disable_model_invocation.as_str(), "true" | "false"), + matches!(disable_model_invocation, "true" | "false"), "{} disable-model-invocation must be a boolean scalar", skill.path.display() ); } if let Some(paths) = skill.frontmatter.get("paths") { let path_globs = paths - .lines() - .map(str::trim) - .filter(|line| !line.is_empty()) - .collect::>(); + .as_list_items() + .unwrap_or_else(|| panic!("{} paths must be a YAML list block", skill.path.display())); assert!( - !path_globs.is_empty() - && path_globs - .iter() - .all(|line| line.starts_with("- ") && line.len() > 2), + path_globs.iter().all(|glob| !glob.is_empty()), "{} paths must be a non-empty YAML list of path globs", skill.path.display() ); @@ -264,14 +338,8 @@ fn assert_required_skill_creator_frontmatter(skill: &SkillDoc) { .and_then(|name| name.to_str()) .expect("skill dir should be utf-8"); - let name = skill - .frontmatter - .get("name") - .unwrap_or_else(|| panic!("{} is missing name", skill.path.display())); - let description = skill - .frontmatter - .get("description") - .unwrap_or_else(|| panic!("{} is missing description", skill.path.display())); + let name = required_scalar_field(skill, "name"); + let description = required_scalar_field(skill, "description"); assert_eq!( name, @@ -281,12 +349,8 @@ fn assert_required_skill_creator_frontmatter(skill: &SkillDoc) { ); assert!( is_skill_creator_name(name), - "{} skill name must be hyphen-case lowercase letters, digits, and hyphens", - skill.path.display() - ); - assert!( - !name.starts_with('-') && !name.ends_with('-') && !name.contains("--"), - "{} skill name cannot start/end with hyphen or contain consecutive hyphens", + "{} skill name must be hyphen-case lowercase letters, digits, and hyphens, \ + without leading/trailing/consecutive hyphens", skill.path.display() ); assert!( @@ -310,77 +374,34 @@ fn assert_allowed_frontmatter(skill: &SkillDoc, allowed: &[&str]) { ); } -fn parse_skill_frontmatter(path: &Path, body: &str) -> BTreeMap { - let rest = body - .strip_prefix("---\n") - .unwrap_or_else(|| panic!("{} must start with YAML frontmatter", path.display())); - let (frontmatter, _) = rest - .split_once("\n---\n") - .unwrap_or_else(|| panic!("{} must close YAML frontmatter", path.display())); - let mut fields: BTreeMap = BTreeMap::new(); - let mut last_key: Option = None; - for line in frontmatter.lines().filter(|line| !line.trim().is_empty()) { - if line.starts_with(char::is_whitespace) { - let Some(key) = &last_key else { - panic!( - "{} has indented frontmatter line before any key: {line:?}", - path.display() - ); - }; - let value = fields - .get_mut(key) - .expect("last frontmatter key should be present"); - if !value.is_empty() { - value.push('\n'); - } - value.push_str(line); - continue; - } - let (key, raw_value) = line - .split_once(':') - .unwrap_or_else(|| panic!("{} has invalid frontmatter line {line:?}", path.display())); - assert!( - fields - .insert( - key.to_string(), - unquote_frontmatter_scalar(raw_value.trim()) - ) - .is_none(), - "{} duplicates frontmatter key {key}", - path.display() - ); - last_key = Some(key.to_string()); - } - fields +fn scalar_field<'a>(skill: &'a SkillDoc, field: &str) -> Option<&'a str> { + skill.frontmatter.get(field).map(|value| { + value.as_scalar().unwrap_or_else(|| { + panic!( + "{} frontmatter {field} must be an inline scalar", + skill.path.display() + ) + }) + }) } -fn unquote_frontmatter_scalar(value: &str) -> String { - if let Some(inner) = value.strip_prefix('\'').and_then(|v| v.strip_suffix('\'')) { - inner.to_string() - } else if let Some(inner) = value.strip_prefix('"').and_then(|v| v.strip_suffix('"')) { - inner.to_string() - } else { - value.to_string() - } +fn required_scalar_field<'a>(skill: &'a SkillDoc, field: &str) -> &'a str { + scalar_field(skill, field) + .unwrap_or_else(|| panic!("{} is missing {field}", skill.path.display())) } -fn assert_metadata_budget( - label: &str, - skills: &[SkillDoc], - max_total_chars: usize, - include: impl Fn(&SkillDoc) -> bool, -) { +fn assert_metadata_budget(label: &str, skills: &[SkillDoc], include: impl Fn(&SkillDoc) -> bool) { let total_metadata_chars = skills .iter() .filter(|skill| include(skill)) .map(|skill| { - skill.frontmatter.get("name").map_or(0, String::len) - + skill.frontmatter.get("description").map_or(0, String::len) + required_scalar_field(skill, "name").len() + + required_scalar_field(skill, "description").len() }) .sum::(); assert!( - total_metadata_chars <= max_total_chars, + total_metadata_chars <= MAX_BUNDLED_SKILL_METADATA_CHARS, "{label} skill metadata uses {total_metadata_chars} chars; keep bundled descriptions concise" ); } @@ -397,31 +418,28 @@ fn assert_scalar(field: &str, value: &str, path: &Path) { "{} frontmatter {field} cannot have leading or trailing whitespace", path.display() ); - assert!( - !value.contains(['\n', '\r']), - "{} frontmatter {field} must be a single line", - path.display() - ); } fn is_skill_creator_name(name: &str) -> bool { !name.is_empty() + && !name.starts_with('-') + && !name.ends_with('-') + && !name.contains("--") && name .bytes() .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') } +/// Agents choose skills from metadata alone, so each description must carry +/// an imperative "Use ..." trigger sentence: either leading the description +/// or following a short capability summary (e.g. "Find code by concept ... +/// Use when searching the codebase"). fn has_trigger_language(description: &str) -> bool { - description.contains("Use when") - || description.contains("Use before") - || description.contains("Use to") + description.starts_with("Use ") || description.contains(". Use ") } fn is_cursor_explicit_invoke_only(skill: &SkillDoc) -> bool { - skill - .frontmatter - .get("disable-model-invocation") - .is_some_and(|value| value == "true") + scalar_field(skill, "disable-model-invocation") == Some("true") } fn assert_skill_tree_uses_supported_files(skill_dir: &Path) { @@ -432,47 +450,28 @@ fn assert_skill_tree_uses_supported_files(skill_dir: &Path) { "INSTALLATION_GUIDE.md", "QUICK_REFERENCE.md", ]; - let mut stack = vec![skill_dir.to_path_buf()]; - while let Some(dir) = stack.pop() { - for entry in std::fs::read_dir(&dir) - .unwrap_or_else(|err| panic!("failed to read {}: {err}", dir.display())) - { - let entry = entry.expect("read skill tree entry"); - let path = entry.path(); - let relative = path.strip_prefix(skill_dir).expect("relative skill path"); - let first = relative - .components() - .next() - .and_then(|component| component.as_os_str().to_str()) - .expect("relative component"); - if path.is_dir() { - assert!( - allowed_resource_dirs.contains(&first), - "{} contains unsupported top-level directory {}", - skill_dir.display(), - first - ); - stack.push(path); - continue; - } - - let file_name = path - .file_name() - .and_then(|name| name.to_str()) - .expect("skill file name should be utf-8"); - assert!( - !forbidden_doc_files.contains(&file_name), - "{} contains auxiliary documentation file {}; keep skill folders lean", - skill_dir.display(), - file_name - ); - assert!( - relative == Path::new("SKILL.md") || allowed_resource_dirs.contains(&first), - "{} contains unsupported top-level file {}", - skill_dir.display(), - relative.display() - ); - } + for relative in relative_files_under(skill_dir) { + let first = relative + .components() + .next() + .and_then(|component| component.as_os_str().to_str()) + .expect("relative component"); + let file_name = relative + .file_name() + .and_then(|name| name.to_str()) + .expect("skill file name should be utf-8"); + assert!( + !forbidden_doc_files.contains(&file_name), + "{} contains auxiliary documentation file {}; keep skill folders lean", + skill_dir.display(), + file_name + ); + assert!( + relative == Path::new("SKILL.md") || allowed_resource_dirs.contains(&first), + "{} contains unsupported top-level entry {}", + skill_dir.display(), + relative.display() + ); } }