diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 3b59e908d..e2877349d 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -651,8 +651,19 @@ async fn parse_skill_file( let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?; - let parsed: SkillFrontmatter = - serde_yaml::from_str(&frontmatter).map_err(SkillParseError::InvalidYaml)?; + let parsed: SkillFrontmatter = match serde_yaml::from_str(&frontmatter) { + Ok(parsed) => Ok(parsed), + Err(original_error) => match repair_frontmatter_scalar_fields(&frontmatter) { + // Some third-party skills use prose like `description: Build for AWS: ECS` + // or `argument-hint: `. Keep the repair line-oriented + // so unrelated invalid YAML still surfaces. + Some(repaired_frontmatter) => { + serde_yaml::from_str(&repaired_frontmatter).map_err(|_| original_error) + } + None => Err(original_error), + }, + } + .map_err(SkillParseError::InvalidYaml)?; let base_name = parsed .name @@ -997,6 +1008,91 @@ fn sanitize_single_line(raw: &str) -> String { raw.split_whitespace().collect::>().join(" ") } +fn repair_frontmatter_scalar_fields(frontmatter: &str) -> Option { + let mut changed = false; + let mut block_scalar_indent: Option = None; + let mut repaired_lines: Vec = Vec::new(); + for line in frontmatter.lines() { + let indent = line + .chars() + .take_while(|character| *character == ' ') + .count(); + if let Some(block_indent) = block_scalar_indent { + if line.trim().is_empty() || indent > block_indent { + repaired_lines.push(line.to_string()); + continue; + } + block_scalar_indent = None; + } + + let Some((key, value)) = line.split_once(':') else { + repaired_lines.push(line.to_string()); + continue; + }; + if key.trim().is_empty() || !value.chars().next().is_none_or(char::is_whitespace) { + repaired_lines.push(line.to_string()); + continue; + } + + let trimmed_start = value.trim_start(); + let leading_whitespace = &value[..value.len() - trimmed_start.len()]; + let mut scalar = trimmed_start; + let mut comment = ""; + for (index, character) in trimmed_start.char_indices() { + if character == '#' + && (index == 0 + || trimmed_start[..index] + .chars() + .next_back() + .is_some_and(char::is_whitespace)) + { + let comment_start = trimmed_start[..index].trim_end().len(); + scalar = &trimmed_start[..comment_start]; + comment = &trimmed_start[comment_start..]; + break; + } + } + + let scalar = scalar.trim_end(); + let Some(first_char) = scalar.chars().next() else { + repaired_lines.push(line.to_string()); + continue; + }; + if matches!(first_char, '|' | '>') { + block_scalar_indent = Some(indent); + repaired_lines.push(line.to_string()); + continue; + } + if matches!(first_char, '\'' | '"') { + repaired_lines.push(line.to_string()); + continue; + } + let mut has_colon_separator = false; + let mut chars = scalar.chars().peekable(); + while let Some(character) = chars.next() { + if character == ':' + && matches!(chars.peek(), Some(next_character) if next_character.is_whitespace()) + { + has_colon_separator = true; + break; + } + } + let invalid_flow_like_scalar = matches!(first_char, '[' | '{' | '@' | '`') + && serde_yaml::from_str::(scalar).is_err(); + if !has_colon_separator && !invalid_flow_like_scalar { + repaired_lines.push(line.to_string()); + continue; + } + + let quoted_scalar = format!("'{}'", scalar.replace('\'', "''")); + repaired_lines.push(format!( + "{key}:{leading_whitespace}{quoted_scalar}{comment}" + )); + changed = true; + } + changed.then(|| repaired_lines.join("\n")) +} + fn validate_len( value: &str, max_len: usize, diff --git a/codex-rs/core-skills/src/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs index f6c71b2be..cc1d8c8e0 100644 --- a/codex-rs/core-skills/src/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -1411,6 +1411,134 @@ async fn loads_short_description_from_metadata() { ); } +#[tokio::test] +async fn loads_unquoted_description_containing_colon_space() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_raw_skill_at( + &codex_home.path().join("skills"), + "colon-description", + "name: colon-description\ndescription: AWS deployment patterns: ECS Fargate, Lambda, and S3", + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg).await; + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "colon-description".to_string(), + description: "AWS deployment patterns: ECS Fargate, Lambda, and S3".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: normalized(&skill_path), + scope: SkillScope::User, + plugin_id: None, + }] + ); +} + +#[tokio::test] +async fn loads_unquoted_short_description_containing_colon_space_and_apostrophe() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_raw_skill_at( + &codex_home.path().join("skills"), + "colon-short-description", + "name: colon-short-description\ndescription: long description\nmetadata:\n short-description: What's included: builds and tests", + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg).await; + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "colon-short-description".to_string(), + description: "long description".to_string(), + short_description: Some("What's included: builds and tests".to_string()), + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: normalized(&skill_path), + scope: SkillScope::User, + plugin_id: None, + }] + ); +} + +#[tokio::test] +async fn loads_unrecognized_frontmatter_fields_that_need_quotes() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_raw_skill_at( + &codex_home.path().join("skills"), + "repaired-unknown-fields", + "name: repaired-unknown-fields\ndescription: valid description\nargument-hint: \ntags: [next,@supabase/ssr]", + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg).await; + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "repaired-unknown-fields".to_string(), + description: "valid description".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: normalized(&skill_path), + scope: SkillScope::User, + plugin_id: None, + }] + ); +} + +#[tokio::test] +async fn preserves_block_scalar_body_while_repairing_other_fields() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_raw_skill_at( + &codex_home.path().join("skills"), + "block-description-with-repair", + "name: block-description-with-repair\ndescription: |-\n Build for AWS: ECS\nargument-hint: ", + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills_for_test(&cfg).await; + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "block-description-with-repair".to_string(), + description: "Build for AWS: ECS".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: normalized(&skill_path), + scope: SkillScope::User, + plugin_id: None, + }] + ); +} + #[tokio::test] async fn enforces_short_description_length_limits() { let codex_home = tempfile::tempdir().expect("tempdir");