Skip to content

fix(skills): change here-now skill frontmatter name from dotted to hyphenated (#53382) - #53447

Open
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/53382-here-now-frontmatter-rev
Open

fix(skills): change here-now skill frontmatter name from dotted to hyphenated (#53382)#53447
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/53382-here-now-frontmatter-rev

Conversation

@Tranquil-Flow

Copy link
Copy Markdown
Contributor

What

The optional here-now skill declared name: here.now in its YAML frontmatter. The skill-name validator (UrlSource._VALID_NAME_RE = ^[a-z][a-z0-9_-]*$) rejects dots, so the skill failed to load/validate with:

name contains invalid characters (must be lowercase a-z, 0-9, hyphens only)

Changed name: here.nowname: here-now. The description, docs links, tags, and user-facing product name still use here.now where appropriate — only the frontmatter identifier changed.

Why

here.now (with a dot) cannot pass the strict skill-name regex on any code path, so the skill could never load. here-now (hyphenated) is valid.

How verified

  • Regression test tests/skills/test_here_now_name.py parses the real frontmatter and validates it through the real project validator (UrlSource._is_valid_skill_name).
  • RED phase: test fails on unpatched main (here.now fails the validator).
  • GREEN phase: test passes after the one-character fix.
  • No other files changed; author: here.now and product references are untouched.

Closes #53382.


Auto-published by Moonsong via Path B automated pipeline.

@alt-glitch alt-glitch added type/bug Something isn't working tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have labels Jun 27, 2026

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved

Trivial frontmatter fix: here.now -> here-now to match the skill name validator regex. The regression test validates the real SKILL.md through the real UrlSource._is_valid_skill_name validator.


Reviewed by Hermes Agent

@teknium1 teknium1 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.

Thanks for the focused frontmatter correction. Current main still has name: here.now at optional-skills/productivity/here-now/SKILL.md:2, while UrlSource._is_valid_skill_name() applies the dot-excluding regex at tools/skills_hub.py:1550-1559; changing the identifier to here-now is aligned with that contract.

Problems

  • The added test is named tests/skills/test_here_now_name.py; AGENTS.md:948-950 requires the tests/skills/test_<skill>_skill.py naming convention.
  • tests/skills/test_here_now_name.py:25 imports yaml directly and duplicates frontmatter parsing. The production parser is agent.skill_utils.parse_frontmatter() at agent/skill_utils.py:123-157; reusing it also follows the skill-test dependency rule in AGENTS.md:948-950.

Suggested changes

  • Rename the test to tests/skills/test_here_now_skill.py.
  • Parse the fixture with agent.skill_utils.parse_frontmatter() before passing its name to UrlSource._is_valid_skill_name().

Automated hermes-sweeper review.

Comment thread tests/skills/test_here_now_name.py Outdated
def _parse_frontmatter(content: str) -> dict:
"""Extract YAML frontmatter between ``---`` markers."""
if not content.startswith("---"):
return {}

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.

Please use agent.skill_utils.parse_frontmatter() instead of importing yaml and maintaining a second parser here. That exercises the shared parser and keeps this skill test within the dependency convention in AGENTS.md:948-950.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

here-now skill uses invalid frontmatter name 'here.now'

4 participants