Skip to content

fix(skills): prevent frontmatter name from diverging from directory name - #21815

Closed
akimuz wants to merge 1 commit into
NousResearch:mainfrom
akimuz:fix/21782-skill-name-frontmatter-mismatch
Closed

fix(skills): prevent frontmatter name from diverging from directory name#21815
akimuz wants to merge 1 commit into
NousResearch:mainfrom
akimuz:fix/21782-skill-name-frontmatter-mismatch

Conversation

@akimuz

@akimuz akimuz commented May 8, 2026

Copy link
Copy Markdown

Related Issue

Fixes #21782

What changed and why

skill_manage(action="create", name="foo", content="---\nname: bar\n...") silently
created a skill whose directory is foo/ but whose skills_list() display name is
bar. The write path never validated that the frontmatter name matches the name
parameter, introducing a dual-namespace problem:

  • skills_list() shows the frontmatter name → bar
  • skill_view("bar")not found (directory-name lookup)
  • skill_manage(action="edit", name="foo")found (directory-name lookup)

Same divergence could be introduced via edit or patch.

Previous fixes (#17914, #18872, #5433, #7669) patched read-path symptoms
(skill_view can't resolve frontmatter name, bundled skill misclassified, etc.).
This PR fixes the write-path root cause: _validate_frontmatter() now accepts an
expected_name parameter and cross-checks it against the frontmatter name field.
All three write paths (_create_skill, _edit_skill, _patch_skill) wire the
directory name through so mismatches are explicitly rejected with an actionable
error message, rather than silently accepted.

Design choice: explicit rejection over silent auto-overwrite — the error message
lets the agent self-correct on the next turn.

How to test

Reproduction (without fix):

skill_manage(
    action="create",
    name="local-paper-summarizer",
    content="---\nname: local-paper-summary\ndescription: ...\n---\n\nSteps..."
)
# skills_list() shows "local-paper-summary"
# skill_view("local-paper-summary") → not found

With fixmismatch rejected:
skill_manage(
    action="create",
    name="local-paper-summarizer",
    content="---\nname: local-paper-summary\ndescription: ...\n---\n\nSteps..."
)
# → "Frontmatter name 'local-paper-summary' does not match skill name
#    'local-paper-summarizer'. The frontmatter name must match the 'name' parameter."

Unit tests:
pytest tests/tools/test_skill_manager_tool.py -v

New test cases:
- create with frontmatter namename paramfail
- create with frontmatter name == name paramsucceed
- edit changing frontmatter name away from directory namefail
- patch altering name: to diverge from directory namefail

Platforms tested

- macOS (Darwin 24.2.0, Python 3.11)

Related

- #17914 / #18872 — read-path: skill_view vs skills_list name mismatch
- #5433 — read-path: bundled manifest vs frontmatter name causes misclassification
- #7669 — read-path: _get_skill_type() vs _scan_skill_names() name mismatch

skill_manage(action=create/edit/patch) silently allowed the
frontmatter  field to differ from the  parameter, causing
skills_list() to display a different name than what skill_view() and
skill_manage look up by directory. Add  cross-check to
_validate_frontmatter() and wire it through all three write paths so
mismatches are explicitly rejected with an actionable error message.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) labels May 8, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competing PR: #21806 addresses the same root cause (fixes #21782) with a broader scope (also touches whatsapp.py, file_tools.py). Both add expected_name validation to _validate_frontmatter(). Maintainers should pick one.

@akimuz

akimuz commented May 8, 2026

Copy link
Copy Markdown
Author

Competing PR: #21806 addresses the same root cause (fixes #21782) with a broader scope (also touches whatsapp.py, file_tools.py). Both add expected_name validation to _validate_frontmatter(). Maintainers should pick one.
Thanks for the clarification
Since #21806 covers the same invariant and extends to more downstream paths, I'll close this PR now
Thanks again for the review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: _create_skill / _edit_skill allow frontmatter name to diverge from directory name

2 participants