Skip to content

fix(persona): pack validate errors on skills the runtime would silently skip - #3532

Open
vedantggwp wants to merge 1 commit into
block:mainfrom
vedantggwp:fix/pack-validate-skill-metadata
Open

fix(persona): pack validate errors on skills the runtime would silently skip#3532
vedantggwp wants to merge 1 commit into
block:mainfrom
vedantggwp:fix/pack-validate-skill-metadata

Conversation

@vedantggwp

@vedantggwp vedantggwp commented Jul 29, 2026

Copy link
Copy Markdown

Implements the PF-2 remainder from PERSONA_PACK_SPEC.md section 16. No issue filed since
the spec documents this as remaining validate work, but happy to split one out if you
prefer tracking it that way. No duplicate found in open PRs.

The runtime silently skips any skill whose SKILL.md lacks name: or description:
(parse_frontmatter returns None, no fallback to the directory name). And a persona
skills: entry pointing at a directory that doesn't exist is silently dropped during
validation's collection pass. So buzz pack validate currently says Valid. for packs
that will quietly lose skills at deploy time. That's the worst kind of pass.

With this change, validate errors on:

  • a persona skills: entry that doesn't resolve to a directory under skills/
  • a skill directory missing SKILL.md
  • SKILL.md with no frontmatter, or frontmatter that isn't valid YAML
  • frontmatter missing name: or description: (or either being empty)

Unchanged: name-vs-directory mismatch stays a warning, same as before. Hooks path
verification (the other half of PF-2's remainder) is left for a follow-up to keep this
diff reviewable.

Tests cover each new error path plus one asserting the mismatch warning didn't get
promoted to an error.

DCO signed. AI-assisted, human-reviewed line by line.

What operators see

Real output from buzz pack validate built off this branch, against a pack with one bad
skills reference and one SKILL.md missing its description:

  ERROR: persona "t": skills entry "skills/nope" does not resolve to a directory under skills/
  WARN:  skill skills/code-review: name "code_review" differs from directory name "code-review"
  ERROR: skill skills/code-review: frontmatter is missing required field "description"

And the no-frontmatter case:

  ERROR: skill skills/code-review: SKILL.md has no frontmatter; the agent runtime silently skips skills without `name` and `description`

Each new error path has a matching unit test in validate.rs
(validate_pack_skill_ref_missing_dir_errors, validate_pack_skill_missing_name_errors,
validate_pack_skill_missing_description_errors, validate_pack_skill_no_frontmatter_errors),
and the pre-existing validate_pack_skill_name_mismatch_warns pins the mismatch case to
a warning so this change can't silently promote it.

…ly skip

Signed-off-by: Vedant Gawande <iamvedant7@gmail.com>
@vedantggwp
vedantggwp requested a review from a team as a code owner July 29, 2026 13:28
Copilot AI review requested due to automatic review settings July 29, 2026 13:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Chessing234 Chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good PF-2 validate tighten-up. Surfacing missing name:/description: and missing skill directories as hard validate errors matches what the runtime already silently drops — better to fail the pack than ship a persona that quietly loses skills. Would be nice to include a tiny fixture-based test case in the PR description (or point at the ones in the diff) so reviewers can see the exact error strings operators will hit.

@vedantggwp

Copy link
Copy Markdown
Author

Good call. Added a "What operators see" section to the description with real output from a build off this branch, plus pointers to the tests covering each error path. Included the case where the naming warning shows up alongside the new errors, since that's the behaviour most worth eyeballing.

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.

3 participants