Skip to content

fix(skills): check directory name against hub lock to protect non-ASCII SKILL.md names from curator#19297

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/curator-non-ascii-hub-skill-name
Closed

fix(skills): check directory name against hub lock to protect non-ASCII SKILL.md names from curator#19297
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/curator-non-ascii-hub-skill-name

Conversation

@liuhao1024
Copy link
Copy Markdown
Contributor

Summary

list_agent_created_skill_names() compares each SKILL.md's parsed name: field against the hub lock keys to determine provenance. When a hub-installed skill has a non-ASCII name: (e.g. Get笔记) but the lock key is an ASCII slug (e.g. getnote), the comparison misses and the curator treats the skill as agent-created — making it eligible for consolidation and archival.

Root Cause

In tools/skill_usage.py::list_agent_created_skill_names():

name = _read_skill_name(skill_md, fallback=skill_md.parent.name)
if name in off_limits:
    continue

_read_hub_installed_names() returns lock keys ({"getnote", ...}), but _read_skill_name() returns the SKILL.md name: field ("Get笔记"). The mismatch lets the skill pass the filter.

Fix

Also check the skill directory name (which matches the hub slug) against off_limits:

if name in off_limits or skill_md.parent.name in off_limits:
    continue

Regression Coverage

  • New test: test_agent_created_excludes_hub_installed_non_ascii_name — creates a hub-installed skill with non-ASCII SKILL.md name and verifies it is excluded from agent-created lists.
  • All 39 existing tests pass (no behavioral change for ASCII-named skills).

Testing

python -m pytest tests/tools/test_skill_usage.py -x -q
# 39 passed in 1.78s

…II SKILL.md names from curator

When a hub-installed skill has a non-ASCII `name:` in its SKILL.md (e.g.
"Get笔记") but the hub lock key is an ASCII slug (e.g. "getnote"), the
curator's `list_agent_created_skill_names()` compared only the parsed
name field against the lock keys.  The mismatch caused hub-installed
skills to be classified as agent-created and eligible for consolidation.

Fix: also compare the skill directory name (which matches the hub slug)
against the off-limits set.

Regression test included.

Fixes Curator misclassifies hub-installed skills with non-ASCII `name` as agent-created NousResearch#19293
@alt-glitch alt-glitch added type/bug Something isn't working tool/skills Skills system (list, view, manage) P2 Medium — degraded but workaround exists labels May 3, 2026
@teknium1
Copy link
Copy Markdown
Contributor

teknium1 commented May 4, 2026

Closing — your fix to `list_agent_created_skill_names()` is subsumed by the current state of main. Here's the lineage:

  1. PR fix(skills): keep manual skills out of curator #19618 (salvaged from @LeonSGP43's fix(skills): keep manual skills out of curator #19235) rewrote `list_agent_created_skill_names()` to require an explicit `created_by: "agent"` provenance marker instead of "not bundled AND not hub-installed". Hub-installed skills don't get that marker, so `Get笔记` is already excluded from the automatic curator path on current main.
  2. PR fix(curator): only mark agent-created for background-review sediment #19621 tightened fix(skills): keep manual skills out of curator #19618 further so only the background self-improvement review fork's `skill_manage(create)` sets the marker — user-directed foreground creates stay unmarked too.

That means your exact fix is dead code against current `list_agent_created_skill_names()` — the function now gates on a different signal.

However, your diagnosis still applies to a sibling function that wasn't in the scope of #19618/#19621: `is_agent_created(skill_name)` (line 210 on main). It still does the original `skill_name not in (bundled ∪ hub_installed)` check, which is narrow enough to survive the provenance-marker refactor. It's consulted by `hermes curator archive/restore` commands and `_mutate()` — narrower blast radius than the automatic curator, but still real. I've opened #19641 to track that with your PR linked for credit, and the fix approach you took here (comparing directory name against the lock keys) is the right one for that function too.

Thanks for the precise diagnosis and the minimal fix — credit preserved in #19641 for whoever picks it up.

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.

3 participants