feat(skills): per-profile filter on dashboard skills page - #29323
Closed
cypres0099 wants to merge 1 commit into
Closed
feat(skills): per-profile filter on dashboard skills page#29323cypres0099 wants to merge 1 commit into
cypres0099 wants to merge 1 commit into
Conversation
## What
Adds a profile filter to the dashboard's Skills page, mirroring the
``?profile=`` convention already established for cron jobs in
``CronPage.tsx`` / ``GET /api/cron/jobs``.
- ``GET /api/skills`` gains an optional ``?profile=`` query param:
- omitted → active-profile skills via the legacy code path (unchanged
backwards-compatible behavior).
- ``?profile=all`` → union across every installed profile, each row
tagged ``profile=<name>``.
- ``?profile=<name>`` → just that profile's skills, tagged with that name.
- ``PUT /api/skills/toggle`` gains the same ``?profile=`` param to target
a specific profile's ``config.yaml`` (omitted falls back to the active
profile, again preserving the old behavior).
- ``SkillsPage`` adds a cron-style ``<Select>`` filter (label "Profile",
options "All profiles" + each installed profile name). Hidden when only
one profile is installed to keep the default-only-install dashboard
visually clean.
- ``SkillRow`` shows a small badge with the source profile name when
viewing "All profiles" so users can tell which profile each row
belongs to. Hidden in single-profile mode where the attribution is
redundant.
## Why
The dashboard previously showed only the active profile's skills (whichever
``HERMES_HOME`` the dashboard process was launched under), with no UI for
toggling skills in other installed profiles. Users running multiple
profiles (default + a worker, a specialist sub-profile, etc.) had to spawn
a second dashboard daemon per profile to manage their skills, which is
operationally awkward.
The CLI's ``hermes skills list --source ...`` and the cron jobs page have
both grown profile-awareness already; this brings the skills page in line.
## Implementation
Backend (``hermes_cli/web_server.py``):
- New helpers (mirror existing ``_cron_*`` patterns):
- ``_skill_profile_home(profile)`` — validate + canonicalize a profile
name to ``(name, HERMES_HOME)``. Raises 400 on invalid names, 404 on
nonexistent profiles. Direct parallel to ``_cron_profile_home``.
- ``_load_profile_disabled_skills(profile_dir)`` /
``_save_profile_disabled_skills(profile_dir, disabled)`` — read/write
``skills.disabled`` in a profile's ``config.yaml`` directly. The
process-level ``load_config`` / ``save_config`` helpers are bound to
the dashboard's own ``HERMES_HOME`` and can't be reused for
cross-profile mutation without invasive global-state changes.
- ``_find_skills_in_profile_dir(profile_dir)`` — scanner that reuses
``tools.skills_tool``'s frontmatter parsing + platform matching against
a specific profile's ``skills/`` directory. ``external_dirs`` are
intentionally not scanned for non-active profiles (the dropdown
targets profile-installed skills; the gateway still honors
``skills.external_dirs`` at runtime).
- ``_list_skills_for_profile(name, profile_dir)`` — orchestrator that
annotates each row with ``profile`` and ``enabled``.
- ``_skills_profile_dicts()`` — inlined parallel of ``_cron_profile_dicts``
so the all-profiles loop has somewhere to read from.
Frontend (``web/src/lib/api.ts``, ``web/src/pages/SkillsPage.tsx``):
- ``api.getSkills(profile = "all")`` — always passes ``?profile=`` (the
legacy bare-call path is preserved server-side for external callers).
- ``api.toggleSkill(name, enabled, profile?)`` — optional third arg
routes the toggle to the named profile's ``config.yaml``.
- ``SkillInfo`` gains optional ``profile?: string`` (back-compat).
- ``SkillsPage`` loads ``profiles`` once on mount, refetches ``skills``
whenever ``selectedProfile`` changes, and routes toggles through the
per-row profile so "All profiles" mode toggles the right (profile, skill)
pair.
## Tests
``tests/hermes_cli/test_web_server.py``:
- ``test_skills_list_for_named_profile`` — ``?profile=<name>`` scans the
profile's own ``skills/`` directory and tags rows with the profile.
- ``test_skills_list_all_unions_across_profiles`` — ``?profile=all``
returns rows from every profile that has a ``skills/`` directory,
each tagged with its source profile.
- ``test_skill_toggle_with_profile_writes_to_profile_config`` —
``PUT /api/skills/toggle?profile=<name>`` writes to that profile's
``config.yaml.skills.disabled``, leaving the dashboard's active
profile untouched. Includes re-enable round-trip.
- ``test_skills_unknown_profile_404`` /
``test_skills_invalid_profile_name_400`` — profile name validation.
All 10 skills-related tests pass; the existing
``test_skills_list_includes_disabled_skills`` is preserved (legacy
``GET /api/skills`` with no ``?profile=`` is unchanged).
3 pre-existing ``TestPtyWebSocket`` failures unrelated to this change
(starlette testclient ``KeyError: 'bytes'`` on websocket receive,
reproduces on plain ``upstream/main``).
Frontend: zero new TypeScript errors or lint warnings on touched files.
## Scope
Single concern: per-profile filtering for the dashboard's skills page.
Out of scope (separate follow-ups, if wanted):
- A reusable ``_dashboard_profile_dicts`` helper that ``_cron_profile_dicts``
and ``_skills_profile_dicts`` both consume. The current duplication is
~5 lines; refactoring would expand the diff for no immediate user-facing
value.
- Per-profile scanning of ``skills.external_dirs`` (currently honored only
for the active profile). Could surface in a follow-up if users ask.
- A "HUB / BUILTIN / LOCAL" source annotation per row — see NousResearch#29312 for a
separate PR that adds that signal.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Contributor
Author
|
Closing as superseded by the profile-scoped skills/toolsets management flow that has since landed on current main. The final upstream route/UI shape differs from this branch, so there is no value in keeping this stale alternate implementation open. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a profile filter to the dashboard's Skills page, mirroring the
?profile=convention already established for cron jobs inCronPage.tsx/GET /api/cron/jobs.GET /api/skillsgains an optional?profile=query param:backwards-compatible behavior).
?profile=all→ union across every installed profile, each rowtagged
profile=<name>.?profile=<name>→ just that profile's skills, tagged with that name.PUT /api/skills/togglegains the same?profile=param to targeta specific profile's
config.yaml(omitted falls back to the activeprofile, again preserving the old behavior).
SkillsPageadds a cron-style<Select>filter (label "Profile",options "All profiles" + each installed profile name). Hidden when only
one profile is installed to keep the default-only-install dashboard
visually clean.
SkillRowshows a small badge with the source profile name whenviewing "All profiles" so users can tell which profile each row
belongs to. Hidden in single-profile mode where the attribution is
redundant.
Why
The dashboard previously showed only the active profile's skills (whichever
HERMES_HOMEthe dashboard process was launched under), with no UI fortoggling skills in other installed profiles. Users running multiple
profiles (default + a worker, a specialist sub-profile, etc.) had to spawn
a second dashboard daemon per profile to manage their skills, which is
operationally awkward.
The CLI's
hermes skills list --source ...and the cron jobs page haveboth grown profile-awareness already; this brings the skills page in line.
Implementation
Backend (
hermes_cli/web_server.py):_cron_*patterns):_skill_profile_home(profile)— validate + canonicalize a profilename to
(name, HERMES_HOME). Raises 400 on invalid names, 404 onnonexistent profiles. Direct parallel to
_cron_profile_home._load_profile_disabled_skills(profile_dir)/_save_profile_disabled_skills(profile_dir, disabled)— read/writeskills.disabledin a profile'sconfig.yamldirectly. Theprocess-level
load_config/save_confighelpers are bound tothe dashboard's own
HERMES_HOMEand can't be reused forcross-profile mutation without invasive global-state changes.
_find_skills_in_profile_dir(profile_dir)— scanner that reusestools.skills_tool's frontmatter parsing + platform matching againsta specific profile's
skills/directory.external_dirsareintentionally not scanned for non-active profiles (the dropdown
targets profile-installed skills; the gateway still honors
skills.external_dirsat runtime)._list_skills_for_profile(name, profile_dir)— orchestrator thatannotates each row with
profileandenabled._skills_profile_dicts()— inlined parallel of_cron_profile_dictsso the all-profiles loop has somewhere to read from.
Frontend (
web/src/lib/api.ts,web/src/pages/SkillsPage.tsx):api.getSkills(profile = "all")— always passes?profile=(thelegacy bare-call path is preserved server-side for external callers).
api.toggleSkill(name, enabled, profile?)— optional third argroutes the toggle to the named profile's
config.yaml.SkillInfogains optionalprofile?: string(back-compat).SkillsPageloadsprofilesonce on mount, refetchesskillswhenever
selectedProfilechanges, and routes toggles through theper-row profile so "All profiles" mode toggles the right (profile, skill)
pair.
Tests
tests/hermes_cli/test_web_server.py:test_skills_list_for_named_profile—?profile=<name>scans theprofile's own
skills/directory and tags rows with the profile.test_skills_list_all_unions_across_profiles—?profile=allreturns rows from every profile that has a
skills/directory,each tagged with its source profile.
test_skill_toggle_with_profile_writes_to_profile_config—PUT /api/skills/toggle?profile=<name>writes to that profile'sconfig.yaml.skills.disabled, leaving the dashboard's activeprofile untouched. Includes re-enable round-trip.
test_skills_unknown_profile_404/test_skills_invalid_profile_name_400— profile name validation.All 10 skills-related tests pass; the existing
test_skills_list_includes_disabled_skillsis preserved (legacyGET /api/skillswith no?profile=is unchanged).3 pre-existing
TestPtyWebSocketfailures unrelated to this change(starlette testclient
KeyError: 'bytes'on websocket receive,reproduces on plain
upstream/main).Frontend: zero new TypeScript errors or lint warnings on touched files.
Scope
Single concern: per-profile filtering for the dashboard's skills page.
Out of scope (separate follow-ups, if wanted):
_dashboard_profile_dictshelper that_cron_profile_dictsand
_skills_profile_dictsboth consume. The current duplication is~5 lines; refactoring would expand the diff for no immediate user-facing
value.
skills.external_dirs(currently honored onlyfor the active profile). Could surface in a follow-up if users ask.
separate PR that adds that signal.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com