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..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,7 +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_DIVERGENCES: &[&str] = &["curating-project-memory"]; + 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_FRONTMATTER_DIVERGENCES: &[&str] = &["running-impacted-tests"]; let root = repo_root(); for &skill in crate::hooks::CURSOR_PLUGIN_SKILLS { let codex_path = root @@ -1620,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( @@ -1631,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/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/plugin_skill_contract_test.rs b/tests/plugin_skill_contract_test.rs new file mode 100644 index 00000000..a65c28d9 --- /dev/null +++ b/tests/plugin_skill_contract_test.rs @@ -0,0 +1,492 @@ +//! 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_BUNDLED_SKILL_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", +]; + +/// 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, +} + +#[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_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 source_root = skills_source_root(CODEX_SKILL_ROOT); + let installed_root = home.path().join("plugins/tracedecay/skills"); + assert_eq!( + skill_dir_names(&installed_root), + skill_dir_names(&source_root), + "generated Codex plugin bundle must ship the same skills as the source bundle" + ); + assert_skill_trees_byte_identical(&source_root, &installed_root); +} + +#[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_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 source_root = skills_source_root(CURSOR_SKILL_ROOT); + let installed_root = home.path().join(".cursor/plugins/local/tracedecay/skills"); + assert_eq!( + skill_dir_names(&installed_root), + skill_dir_names(&source_root), + "generated Cursor plugin bundle must ship the same skills as the source bundle" + ); + assert_skill_trees_byte_identical(&source_root, &installed_root); +} + +#[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, |_| 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 = required_scalar_field(skill, "description"); + 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.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() + ); + + 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 skills_source_root(root: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join(root) +} + +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!( + "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(&body) + .unwrap_or_else(|err| panic!("{}: {err}", path.display())); + SkillDoc { + path, + body, + frontmatter, + } + }) + .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(), + tracedecay_bin: "/tmp/tracedecay-test-bin".to_string(), + tool_permissions: expected_tool_perms(), + profile: None, + project_root: None, + dashboard: true, + } +} + +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 directory name should be utf-8") + .to_string() + }) + .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 = required_scalar_field(skill, "description"); + 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) = scalar_field(skill, "disable-model-invocation") { + assert!( + 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 + .as_list_items() + .unwrap_or_else(|| panic!("{} paths must be a YAML list block", skill.path.display())); + assert!( + path_globs.iter().all(|glob| !glob.is_empty()), + "{} 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 = required_scalar_field(skill, "name"); + let description = required_scalar_field(skill, "description"); + + 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, \ + without leading/trailing/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 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 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], include: impl Fn(&SkillDoc) -> bool) { + let total_metadata_chars = skills + .iter() + .filter(|skill| include(skill)) + .map(|skill| { + required_scalar_field(skill, "name").len() + + required_scalar_field(skill, "description").len() + }) + .sum::(); + + assert!( + total_metadata_chars <= MAX_BUNDLED_SKILL_METADATA_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() + ); +} + +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.starts_with("Use ") || description.contains(". Use ") +} + +fn is_cursor_explicit_invoke_only(skill: &SkillDoc) -> bool { + scalar_field(skill, "disable-model-invocation") == Some("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", + ]; + 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() + ); + } +} + +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() + ); + } +}