Skip to content

feat(skills): find & diff user-modified bundled skills (salvage of #47802) - #48286

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/skills-list-modified-diff
Jun 18, 2026
Merged

feat(skills): find & diff user-modified bundled skills (salvage of #47802)#48286
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/skills-list-modified-diff

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

Salvage of #47802 by @xxxigm (credit preserved via cherry-pick). Closes the two gaps reported on Discord (Hana): hermes update says certain bundled skills were "modified by the user" and won't be updated, but there was no way to see which skills or diff them.

Verified against main:

  • Reverting already existshermes skills reset <name> (re-baseline) and --restore (replace with stock). ✅
  • Listing the flagged skills — real gap. ❌ → now hermes skills list-modified
  • Diffing against stock — real gap. ❌ → now hermes skills diff <name>

Pure CLI surface (no model tools, no per-call schema growth). Detection reuses the manifest origin-hash the sync engine already maintains.

Why salvage (and why this over the duplicate #17156)

There's an older PR (#17156) that adds only hermes skills diff, matching the bundled skill by directory name and scanning one category level deep. That breaks for a large fraction of the catalog:

  • 36 of 240 bundled skills have frontmatter-name ≠ dirname (e.g. dir mlops/vllm → name serving-llms-vllm).
  • 36 skills nest >1 category level deep (e.g. mlops/vector-databases/qdrant).

This PR resolves skills via _discover_bundled_skills (frontmatter name) + _compute_relative_dest (full category path) — exactly how the sync engine addresses skills — so it's correct for all 240, and it adds list-modified (the primary reported gap) which #17156 lacks. #17156 is also stale (conflicting, unreviewed). This PR supersedes it.

What the salvage adds on top of @xxxigm's work

Fix Detail
W1 — sibling notice The original patched the ~ N user-modified (kept) notice in the git-pull update path (_cmd_update_impl) to point at hermes skills list-modified, but the byte-identical notice in the unpack/install update path (main.py) was missed. Users hitting that path never learned the command exists. Mirrored the hint to both sites.
W2 — diff verb disambiguation hermes skills diff <name> (bundled-vs-stock) now shares the verb with the gateway write-approval diff <id>. The gateway handler's docstring + chat-truncation message pointed users to /skills diff <id> "on the CLI" — which now resolves a bundled skill by that name instead. Repointed to the pending JSON file and documented that the two diff commands are distinct.
Test Added test_update_modified_notice.py: an invariant test asserting every user-modified (kept) notice in main.py carries the discovery hint (guards the sibling-drift bug class). Mutation-tested — fails when either site loses the hint.

Detection logic (faithful mirror of the sync loop)

list_user_modified_bundled_skills() uses the exact test the sync loop (tools/skills_sync.py) uses to decide what to skip:

origin_hash = manifest.get(name)
if not origin_hash: continue          # un-baselined / v1 → not a tracked modification
if _dir_hash(dest) != origin_hash:    # user edited → flag

Test plan

  • tests/tools/test_skills_list_modified_diff.py — 6 passed (real sync pipeline, no mocked comparison).
  • tests/hermes_cli/test_update_modified_notice.py — invariant guard for both update paths (mutation-tested).
  • tests/hermes_cli/test_skills_hub.py + test_argparse_flag_propagation.py — 45 passed (no parser/routing regressions).
  • Programmatic E2E against a temp HERMES_HOME: pristine → not flagged; edited → listed + diffed; unknown skill → not-ok; reset --restore clears modified state.

Based on #47802 by @xxxigm. Supersedes #17156 by @shashwatgokhe.

xxxigm and others added 3 commits June 18, 2026 12:26
`hermes update` keeps (won't overwrite) bundled skills the user edited
locally, but only printed a count — "~ N user-modified (kept)" — with no way
to learn which skills, or see what changed. Reverting already existed
(`hermes skills reset <name> [--restore]`); discovery and inspection did not.

Add two CLI commands (zero model-tool footprint), reusing the manifest
origin-hash that sync already maintains:

- `hermes skills list-modified [--json]` — list the bundled skills whose
  on-disk copy diverges from the last-synced origin hash (the exact test the
  sync loop uses to decide what to skip).
- `hermes skills diff <name>` — unified diff between the user's copy and the
  current bundled (stock) version, so the user can confirm what changed
  before reverting.

Both are mirrored as `/skills list-modified` and `/skills diff`. The
`hermes update` notice now points at `hermes skills list-modified`. Core
helpers `list_user_modified_bundled_skills()` and `diff_bundled_skill()` live
in tools/skills_sync.py alongside the existing reset logic.
Exercises the real sync pipeline (no mocked comparison logic): a pristine
synced skill is not flagged; an edited one is listed and diffed (modified +
added files); an unknown skill returns not-ok; and `reset --restore` clears
the modified state so revert and discovery stay consistent.
…iguate diff

Salvage follow-up to the cherry-picked feat/test commits:

- W1: the unpack/install update path in main.py printed the
  '~ N user-modified (kept)' notice without the new
  'hermes skills list-modified' hint that the git-pull path got.
  Mirror the hint to both sites so the count is actionable
  regardless of which update path runs.
- W2: 'hermes skills diff <name>' (bundled-vs-stock) now shares the
  verb with the gateway write-approval 'diff <id>'. The gateway
  handler's docstring + truncation message pointed users to
  '/skills diff <id>' on the CLI, which now resolves a bundled skill
  by that name instead. Point at the pending JSON file and note the
  two diff commands are distinct.
- Add an invariant test asserting every 'user-modified (kept)' notice
  in main.py carries the discovery hint (guards sibling drift).
@alt-glitch alt-glitch added type/feature New feature or request comp/cli CLI entry point, hermes_cli/, setup wizard tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have labels Jun 18, 2026
@kshitijk4poor
kshitijk4poor enabled auto-merge June 18, 2026 07:03
@kshitijk4poor
kshitijk4poor merged commit 737007e into NousResearch:main Jun 18, 2026
34 checks passed
kshitijk4poor added a commit that referenced this pull request Jun 18, 2026
refactor(skills): dedupe file-listing + share user-modified predicate (follow-up to #48286)
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…ls-list-modified-diff

feat(skills): find & diff user-modified bundled skills (salvage of NousResearch#47802)
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…-diff-cleanup

refactor(skills): dedupe file-listing + share user-modified predicate (follow-up to NousResearch#48286)
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…ls-list-modified-diff

feat(skills): find & diff user-modified bundled skills (salvage of NousResearch#47802)
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…-diff-cleanup

refactor(skills): dedupe file-listing + share user-modified predicate (follow-up to NousResearch#48286)
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…ls-list-modified-diff

feat(skills): find & diff user-modified bundled skills (salvage of NousResearch#47802)
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…-diff-cleanup

refactor(skills): dedupe file-listing + share user-modified predicate (follow-up to NousResearch#48286)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…ls-list-modified-diff

feat(skills): find & diff user-modified bundled skills (salvage of NousResearch#47802)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…-diff-cleanup

refactor(skills): dedupe file-listing + share user-modified predicate (follow-up to NousResearch#48286)
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…ls-list-modified-diff

feat(skills): find & diff user-modified bundled skills (salvage of NousResearch#47802)
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…-diff-cleanup

refactor(skills): dedupe file-listing + share user-modified predicate (follow-up to NousResearch#48286)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants