mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-17 00:26:41 +03:00
Port: Fix Codex skill version metadata (#703)
Upstream sha 482368511a.
Codex's validator rejects unknown top-level keys, so the Codex and `.agents`
skills now carry `version` under the spec-defined `metadata:` map. Both
version readers learn the same parser: `parse_skill_frontmatter_version` in
`crates/context` (the boot update check) and `extract_version` in
`crates/skills` (`getSkillsVersion`). A metadata version wins, a legacy
top-level one still reads, only the map's own indent level counts, tabs count
as two spaces, and a comment line is skipped.
The build-tooling half (`versionInMetadata` on the two providers, the YAML
emitter's nested-object branch) came in with the merge.
Fourteen frontmatter shapes were recorded from origin/main's
`parseSkillFrontmatterVersion` and pinned as unit tests in both crates.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
This commit is contained in:
Paul Bakaus
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
50684ececb
commit
4d6d57b8bd
@@ -344,14 +344,83 @@ fn build_target_selection_directive(sel: &TargetSelection) -> String {
|
||||
|
||||
// ─── Update check ──────────────────────────────────────────────────────────
|
||||
|
||||
static VERSION_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"(?m)^version:\s*(.+)$").unwrap());
|
||||
static FRONTMATTER_RE: Lazy<Regex> = Lazy::new(|| {
|
||||
Regex::new(r"(?s)^---[ \t]*\r?\n(.*?)\r?\n---(?:[ \t]*\r?\n|[ \t]*$)").unwrap()
|
||||
});
|
||||
static METADATA_KEY_RE: Lazy<Regex> =
|
||||
Lazy::new(|| Regex::new(r"^metadata:\s*(?:#.*)?$").unwrap());
|
||||
static VERSION_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"^version:\s*(.+?)\s*$").unwrap());
|
||||
|
||||
/// JS: context.mjs#parseSkillFrontmatterVersion
|
||||
///
|
||||
/// Codex's validator rejects unknown top-level keys, so the Codex and
|
||||
/// `.agents` skills carry `version` under the spec-defined `metadata:` map
|
||||
/// (#703). A metadata version wins; a legacy top-level one still reads.
|
||||
fn parse_skill_frontmatter_version(content: &str) -> Option<String> {
|
||||
let caps = FRONTMATTER_RE.captures(content)?;
|
||||
let body = caps.get(1)?.as_str();
|
||||
|
||||
let mut metadata_version: Option<String> = None;
|
||||
let mut top_level_version: Option<String> = None;
|
||||
let mut in_metadata = false;
|
||||
let mut metadata_indent: Option<usize> = None;
|
||||
|
||||
for line in body.split('\n') {
|
||||
let line = line.strip_suffix('\r').unwrap_or(line);
|
||||
let trimmed_start = line.trim_start();
|
||||
if trimmed_start.is_empty() || trimmed_start.starts_with('#') {
|
||||
continue;
|
||||
}
|
||||
let indent_text: String = line.chars().take_while(|c| *c == ' ' || *c == '\t').collect();
|
||||
// JS `indentText.replace(/\t/g, ' ').length`.
|
||||
let indent = indent_text.replace('\t', " ").chars().count();
|
||||
|
||||
if indent == 0 {
|
||||
in_metadata = METADATA_KEY_RE.is_match(line);
|
||||
metadata_indent = None;
|
||||
if let Some(m) = VERSION_RE.captures(line) {
|
||||
top_level_version = Some(m[1].to_string());
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if !in_metadata {
|
||||
continue;
|
||||
}
|
||||
if metadata_indent.is_none() {
|
||||
metadata_indent = Some(indent);
|
||||
}
|
||||
if metadata_indent != Some(indent) {
|
||||
continue;
|
||||
}
|
||||
if let Some(m) = VERSION_RE.captures(line.trim()) {
|
||||
metadata_version = Some(m[1].to_string());
|
||||
}
|
||||
}
|
||||
|
||||
let value = metadata_version.or(top_level_version)?;
|
||||
let v = js_trim(&value);
|
||||
if v.is_empty() {
|
||||
return None;
|
||||
}
|
||||
Some(strip_matched_quotes(v))
|
||||
}
|
||||
|
||||
/// JS `.replace(/^(["'])(.*)\1$/, '$2')`: only a matched pair is stripped.
|
||||
fn strip_matched_quotes(v: &str) -> String {
|
||||
let chars: Vec<char> = v.chars().collect();
|
||||
if chars.len() >= 2 {
|
||||
let first = chars[0];
|
||||
if (first == '"' || first == '\'') && chars[chars.len() - 1] == first {
|
||||
return chars[1..chars.len() - 1].iter().collect();
|
||||
}
|
||||
}
|
||||
v.to_string()
|
||||
}
|
||||
|
||||
fn read_local_skill_version(provider: &Provider) -> Option<String> {
|
||||
let p = provider.skill_md_path()?;
|
||||
let content = safe_read(&p)?;
|
||||
let m = VERSION_RE.captures(&content)?;
|
||||
let v = js_trim(&m[1]);
|
||||
Some(strip_one_quote_each_end(v))
|
||||
parse_skill_frontmatter_version(&content)
|
||||
}
|
||||
|
||||
fn update_cache_path(env: &Env) -> String {
|
||||
@@ -619,3 +688,38 @@ pub fn run(args: &[String], io: &mut Io) -> i32 {
|
||||
io.out(&format!("{}\n", parts.join("\n\n---\n\n")));
|
||||
0
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod skill_version_tests {
|
||||
use super::parse_skill_frontmatter_version as v;
|
||||
|
||||
/// Values recorded from origin/main's `parseSkillFrontmatterVersion` (#703).
|
||||
#[test]
|
||||
fn frontmatter_version_shapes() {
|
||||
assert_eq!(v("---\nname: impeccable\nversion: 4.1.3\n---\n\nbody\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(v("---\nname: impeccable\nversion: \"4.1.3\"\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(v("---\nname: impeccable\nversion: '4.1.3'\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(
|
||||
v("---\nname: impeccable\nmetadata:\n version: 4.1.3\n argument-hint: \"[t]\"\n---\n").as_deref(),
|
||||
Some("4.1.3")
|
||||
);
|
||||
// A metadata version wins over a legacy top-level one, in either order.
|
||||
assert_eq!(v("---\nversion: 1.0.0\nmetadata:\n version: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(
|
||||
v("---\nmetadata:\n version: 4.1.3\nname: x\nversion: 2.0.0\n---\n").as_deref(),
|
||||
Some("4.1.3")
|
||||
);
|
||||
// Only the map's own indent level counts, so a deeper key is ignored.
|
||||
assert_eq!(
|
||||
v("---\nmetadata:\n a:\n version: 9.9.9\n version: 4.1.3\n---\n").as_deref(),
|
||||
Some("4.1.3")
|
||||
);
|
||||
assert_eq!(v("---\nmetadata:\n\tversion: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(v("---\nmetadata: # note\n version: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(v("---\r\nmetadata:\r\n version: 4.1.3\r\n---\r\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(v("---\n# version: 9.9.9\nversion: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(v("--- \nversion: 4.1.3\n--- \n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(v("version: 4.1.3\n"), None);
|
||||
assert_eq!(v("---\nversion:\n---\n"), None);
|
||||
}
|
||||
}
|
||||
|
||||
+113
-10
@@ -507,24 +507,104 @@ pub fn is_real_skill_dir(skills_dir: &str, name: &str) -> bool {
|
||||
util::is_real_dir(&full) && util::exists(&jsp::join(&[&full, "SKILL.md"]))
|
||||
}
|
||||
|
||||
/// `^version:\s*(.+)$` (multiline) with surrounding quotes stripped.
|
||||
/// JS: skills.mjs#parseSkillFrontmatterVersion
|
||||
///
|
||||
/// Codex's validator rejects unknown top-level keys, so the Codex and
|
||||
/// `.agents` skills carry `version` under the spec-defined `metadata:` map
|
||||
/// (#703). A metadata version wins; a legacy top-level one still reads.
|
||||
fn extract_version(content: &str) -> Option<String> {
|
||||
for line in content.split('\n') {
|
||||
let body = frontmatter_body(content)?;
|
||||
|
||||
let mut metadata_version: Option<String> = None;
|
||||
let mut top_level_version: Option<String> = None;
|
||||
let mut in_metadata = false;
|
||||
let mut metadata_indent: Option<usize> = None;
|
||||
|
||||
for line in body.split('\n') {
|
||||
let line = line.strip_suffix('\r').unwrap_or(line);
|
||||
if let Some(rest) = line.strip_prefix("version:") {
|
||||
let v = rest.trim_start_matches(|c: char| c.is_whitespace());
|
||||
if v.is_empty() {
|
||||
continue;
|
||||
let trimmed_start = line.trim_start();
|
||||
if trimmed_start.is_empty() || trimmed_start.starts_with('#') {
|
||||
continue;
|
||||
}
|
||||
let indent_text: String = line.chars().take_while(|c| *c == ' ' || *c == '\t').collect();
|
||||
// JS `indentText.replace(/\t/g, ' ').length`.
|
||||
let indent = indent_text.replace('\t', " ").chars().count();
|
||||
|
||||
if indent == 0 {
|
||||
in_metadata = is_metadata_key_line(line);
|
||||
metadata_indent = None;
|
||||
if let Some(v) = version_value(line) {
|
||||
top_level_version = Some(v);
|
||||
}
|
||||
let v = v.trim();
|
||||
let v = v.strip_prefix(['"', '\'']).unwrap_or(v);
|
||||
let v = v.strip_suffix(['"', '\'']).unwrap_or(v);
|
||||
return Some(v.to_string());
|
||||
continue;
|
||||
}
|
||||
if !in_metadata {
|
||||
continue;
|
||||
}
|
||||
if metadata_indent.is_none() {
|
||||
metadata_indent = Some(indent);
|
||||
}
|
||||
if metadata_indent != Some(indent) {
|
||||
continue;
|
||||
}
|
||||
if let Some(v) = version_value(line.trim()) {
|
||||
metadata_version = Some(v);
|
||||
}
|
||||
}
|
||||
|
||||
let value = metadata_version.or(top_level_version)?;
|
||||
let v = value.trim();
|
||||
if v.is_empty() {
|
||||
return None;
|
||||
}
|
||||
// JS `.replace(/^(["'])(.*)\1$/, '$2')`: only a matched pair is stripped.
|
||||
let chars: Vec<char> = v.chars().collect();
|
||||
if chars.len() >= 2 {
|
||||
let first = chars[0];
|
||||
if (first == '"' || first == '\'') && chars[chars.len() - 1] == first {
|
||||
return Some(chars[1..chars.len() - 1].iter().collect());
|
||||
}
|
||||
}
|
||||
Some(v.to_string())
|
||||
}
|
||||
|
||||
/// JS `/^---[ \t]*\r?\n([\s\S]*?)\r?\n---(?:[ \t]*\r?\n|[ \t]*$)/`.
|
||||
fn frontmatter_body(content: &str) -> Option<&str> {
|
||||
let rest = content.strip_prefix("---")?;
|
||||
let rest = rest.trim_start_matches([' ', '\t']);
|
||||
let rest = rest.strip_prefix("\r\n").or_else(|| rest.strip_prefix('\n'))?;
|
||||
let mut from = 0usize;
|
||||
while let Some(idx) = rest[from..].find("\n---") {
|
||||
let at = from + idx;
|
||||
let after = &rest[at + 4..];
|
||||
let tail = after.trim_start_matches([' ', '\t']);
|
||||
if tail.is_empty() || tail.starts_with('\n') || tail.starts_with("\r\n") {
|
||||
let body = &rest[..at];
|
||||
return Some(body.strip_suffix('\r').unwrap_or(body));
|
||||
}
|
||||
from = at + 1;
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// JS `/^metadata:\s*(?:#.*)?$/`.
|
||||
fn is_metadata_key_line(line: &str) -> bool {
|
||||
let Some(rest) = line.strip_prefix("metadata:") else { return false };
|
||||
let rest = rest.trim_start_matches(|c: char| c.is_whitespace());
|
||||
rest.is_empty() || rest.starts_with('#')
|
||||
}
|
||||
|
||||
/// JS `/^version:\s*(.+?)\s*$/` on an already-trimmed line.
|
||||
fn version_value(line: &str) -> Option<String> {
|
||||
let rest = line.strip_prefix("version:")?;
|
||||
let v = rest.trim();
|
||||
if v.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(v.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
/// JS: getFlagValue(flags, name): `--name=value` or `--name value`.
|
||||
pub fn get_flag_value<'a>(flags: &'a [String], name: &str) -> Option<&'a str> {
|
||||
let prefix = format!("{name}=");
|
||||
@@ -680,8 +760,31 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn version_extraction() {
|
||||
// Values recorded from origin/main's parseSkillFrontmatterVersion (#703).
|
||||
assert_eq!(extract_version("---\nname: x\nversion: \"9.9.9\"\n---").as_deref(), Some("9.9.9"));
|
||||
assert_eq!(extract_version("---\nname: x\n---"), None);
|
||||
assert_eq!(extract_version("---\nname: x\nversion: 4.1.3\n---\n\nbody\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(extract_version("---\nname: x\nversion: '4.1.3'\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(
|
||||
extract_version("---\nname: x\nmetadata:\n version: 4.1.3\n argument-hint: \"[t]\"\n---\n").as_deref(),
|
||||
Some("4.1.3")
|
||||
);
|
||||
assert_eq!(extract_version("---\nversion: 1.0.0\nmetadata:\n version: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(
|
||||
extract_version("---\nmetadata:\n version: 4.1.3\nname: x\nversion: 2.0.0\n---\n").as_deref(),
|
||||
Some("4.1.3")
|
||||
);
|
||||
assert_eq!(
|
||||
extract_version("---\nmetadata:\n a:\n version: 9.9.9\n version: 4.1.3\n---\n").as_deref(),
|
||||
Some("4.1.3")
|
||||
);
|
||||
assert_eq!(extract_version("---\nmetadata:\n\tversion: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(extract_version("---\nmetadata: # note\n version: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(extract_version("---\r\nmetadata:\r\n version: 4.1.3\r\n---\r\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(extract_version("---\n# version: 9.9.9\nversion: 4.1.3\n---\n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(extract_version("--- \nversion: 4.1.3\n--- \n").as_deref(), Some("4.1.3"));
|
||||
assert_eq!(extract_version("version: 4.1.3\n"), None);
|
||||
assert_eq!(extract_version("---\nversion:\n---\n"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user