Skip to content

fix: quote review-code skill argument hint#616

Merged
johnnygreco merged 1 commit into
mainfrom
johnny/fix/review-code-skill-yaml
May 7, 2026
Merged

fix: quote review-code skill argument hint#616
johnnygreco merged 1 commit into
mainfrom
johnny/fix/review-code-skill-yaml

Conversation

@johnnygreco
Copy link
Copy Markdown
Contributor

📋 Summary

This PR fixes the review-code skill frontmatter so Codex can load it successfully. Quoting the argument-hint preserves the Claude Code-style metadata while making the value valid YAML.

🔗 Related Issue

N/A

🔄 Changes

  • Quote the argument-hint value in .agents/skills/review-code/SKILL.md so [pr-number] [special instructions] is parsed as a string instead of invalid YAML flow syntax.

🧪 Testing

  • Skill frontmatter parses successfully with python3/PyYAML
  • git diff --check origin/main..HEAD passes
  • make test passes (not run — metadata-only skill frontmatter change)
  • Unit tests added/updated (N/A — no runtime code change)
  • E2E tests added/updated (N/A — no E2E behavior change)

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • Architecture docs updated (N/A — skill metadata only)

Signed-off-by: Johnny Greco <jogreco@nvidia.com>
@johnnygreco johnnygreco requested a review from a team as a code owner May 7, 2026 18:47
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR fixes a YAML parsing error in the review-code skill frontmatter by quoting the argument-hint value. Without quotes, [pr-number] [special instructions] is interpreted by YAML parsers as an invalid flow sequence, preventing Codex from loading the skill.

  • Wraps the argument-hint string in double quotes in .agents/skills/review-code/SKILL.md so the value is treated as a plain string instead of a YAML flow sequence.

Confidence Score: 5/5

Safe to merge — the change is a one-line YAML quoting fix with no runtime behavior impact.

The fix correctly addresses a real YAML parsing failure by quoting a string that was being misinterpreted as a flow sequence. The change is isolated to skill metadata and does not affect any runtime logic.

No files require special attention.

Important Files Changed

Filename Overview
.agents/skills/review-code/SKILL.md Single-character fix quoting the argument-hint YAML value to prevent it from being parsed as an invalid flow sequence; no logic changes.

Sequence Diagram

sequenceDiagram
    participant Codex
    participant SKILL.md
    participant YAMLParser

    Codex->>SKILL.md: Load skill frontmatter
    SKILL.md->>YAMLParser: Parse YAML block
    note over YAMLParser: Before: argument-hint: [pr-number] [special instructions]<br/>→ parsed as invalid YAML flow sequence → error
    note over YAMLParser: After: argument-hint: "[pr-number] [special instructions]"<br/>→ parsed as string → success
    YAMLParser-->>Codex: Return skill metadata (string value)
    Codex-->>Codex: Skill loaded successfully
Loading

Reviews (1): Last reviewed commit: "fix: quote review-code skill argument hi..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Review: PR #616 — fix: quote review-code skill argument hint

Summary

One-character-class change to .agents/skills/review-code/SKILL.md: the
argument-hint frontmatter value is wrapped in double quotes so the
bracketed form [pr-number] [special instructions] is parsed as a string
instead of YAML flow sequence syntax. This unblocks Codex loading the skill
while remaining compatible with Claude Code, which uses the same field as
a display hint.

Findings

Correctness

  • YAML flow scalars that start with [ are parsed as the beginning of a
    flow sequence; [pr-number] [special instructions] is not valid flow
    syntax (two sequences with no separator + unquoted words), so strict
    loaders reject it. Double-quoting makes it an unambiguous string — the
    right fix.
  • No functional effect on Claude Code: argument-hint is a free-form
    string display hint, so quoting does not change what the user sees aside
    from the literal value being identical.

Consistency

  • Worth scanning the rest of .agents/skills/**/SKILL.md for the same
    pattern so other skills don't hit the same Codex-load failure later. Not
    required for this PR, but a natural follow-up.

Testing

  • The author notes PyYAML parses the frontmatter successfully, which is
    the right check for a metadata-only change. No runtime code path is
    affected, so skipping make test is reasonable.

Risk

  • Blast radius is a single frontmatter line in a developer-facing skill
    definition. Backward-compatible with existing Claude Code behavior.

Verdict

Approve. Minimal, targeted fix with a clear motivation (YAML-strict loaders
like Codex) and no risk to runtime behavior. Consider a follow-up sweep of
other SKILL.md files to apply the same quoting preemptively.

@johnnygreco johnnygreco merged commit 604fdd9 into main May 7, 2026
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants