Skip to content

Allow slash commands implemented in a hermes-agent plugin to be used …#1940

Closed
plerohellec wants to merge 1 commit into
nesquena:masterfrom
plerohellec:plugin-slash-commands
Closed

Allow slash commands implemented in a hermes-agent plugin to be used …#1940
plerohellec wants to merge 1 commit into
nesquena:masterfrom
plerohellec:plugin-slash-commands

Conversation

@plerohellec
Copy link
Copy Markdown
Contributor

…in hermes-webui.

These is the PR I got from GPT 5.4 for issue #1935. I'm not familiar with the code base so I can't really comment on it. All that I can say is that it fixes the problem in my environment.

@plerohellec plerohellec force-pushed the plugin-slash-commands branch from 0ae3602 to 29d9192 Compare May 8, 2026 23:28
@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Summary

Read the full diff against origin/master, the new helpers in api/commands.py, and the affected blocks in api/routes.py / static/commands.js / static/messages.js on the PR HEAD. Also cross-checked ~/.hermes/hermes-agent/hermes_cli/plugins.py:1255-1322 to verify the plugin-registry contract this hooks into.

The plugin-command intent is right and matches what the agent exposes. Unfortunately the diff also removes a lot of recently-merged unrelated functionality, which I think is a base-mismatch artifact of the codegen rather than something you wanted to do.

What the PR gets right

api/commands.py is the correct integration point. The new list_commands patch and execute_plugin_command use the right agent surface:

# api/commands.py:60-72  (PR HEAD)
from hermes_cli.plugins import get_plugin_commands
plugin_cmds = get_plugin_commands() or {}
existing_names = {c['name'] for c in out}
for cmd_name, cmd_info in plugin_cmds.items():
    if cmd_name in existing_names or cmd_name in _NEVER_EXPOSE:
        continue
    out.append({
        'name': cmd_name,
        'description': str(cmd_info.get('description', 'Plugin command')),
        'category': 'Plugin',
        ...
    })

That matches hermes_cli/plugins.py:1315-1322 (get_plugin_commands() -> Dict[str, dict] keyed by name), and execute_plugin_command correctly uses both get_plugin_command_handler and resolve_plugin_command_result to handle async handlers under inspect.isawaitable. The static/commands.js executeAgentPluginCommand POST to /api/commands/exec is also a reasonable shape.

This kernel of the PR — api/commands.py (+64 lines), the /api/commands/exec POST handler, and the executeAgentPluginCommand JS path — should fix #1935.

Why I can't recommend merging as-is

The diff removes several already-merged features unrelated to plugin slash commands. I think the model that wrote this had an older snapshot of the codebase, so when you applied the diff on top of current master it deleted the newer code. Concretely:

  1. /goal command (feat: add WebUI /goal command support #1866). The PR removes the /api/goal route handler entirely:

    --- master api/routes.py:4272 has:
        if parsed.path == "/api/goal":
            return _handle_goal_command(handler, body)
    --- PR HEAD api/routes.py: gone
    

    static/commands.js also drops the cmdGoal function (50 deleted lines around the _resetAssistantSegment removal), and static/messages.js drops the goal and goal_continue SSE event listeners (~75 lines). That ships feat: add WebUI /goal command support #1866 broken.

  2. Profile-isolated skills helpers (bug(profiles): renamed root/default profile resolves as profiles/<name> #1612 / 6c4b769). Master api/routes.py:93-279 defines _active_skills_dir, _active_skill_search_dirs, _skills_list_from_dir, and _find_skill_in_dir. The PR HEAD's api/routes.py reverts the /api/skills GET handler to the pre-fix shape that imports tools.skills_tool.skills_list directly:

    # PR HEAD api/routes.py:3273-3278
    if parsed.path == "/api/skills":
        from tools.skills_tool import skills_list as _skills_list
        raw = _skills_list()
        data = json.loads(raw) if isinstance(raw, str) else raw
        return j(handler, {"skills": data.get("skills", [])})

    That re-introduces the bug where skills don't honor the per-request profile (the agent module-level tools.skills_tool.SKILLS_DIR points at the server-startup profile, not the cookie-scoped profile).

  3. _profiles_match relocation (feat(mcp): Option A rewrite — import api.models/api.profiles canonically (#1616) #1895). Master api/routes.py:79 does from api.profiles import _profiles_match; the PR HEAD inlines a duplicate definition at api/routes.py:75, undoing the feat(mcp): Option A rewrite — import api.models/api.profiles canonically (#1616) #1895 review fix.

  4. interim_assistant SSE handler. static/messages.js drops the source.addEventListener('interim_assistant', ...) block. That's the visible-mid-turn-assistant-message wiring that issue Show user-visible mid-turn assistant messages in WebUI #1858 is asking for more of, not less.

  5. Request wedge diagnostics. Master has these; PR HEAD doesn't.

The line-count bias confirms it: 207 insertions vs 661 deletions over 5 files. The authentic plugin-command kernel is roughly 64 + ~30 + ~40 = ~135 lines added across api/commands.py, static/commands.js, static/messages.js. Everything else is the model rewriting api/routes.py and the JS files against an older base.

(The empty statusCheckRollup on the PR — no CI runs — also points at the branch shape: it isn't merging cleanly enough for the test workflow to fire, or it raced the workflow trigger.)

Recommendation

Rebase the branch onto current master (0b7e1e60) and discard the model's api/routes.py rewrite and the static/messages.js deletions wholesale. Keep only:

  • api/commands.py additions (the get_plugin_commands extension to list_commands and the new execute_plugin_command).
  • The new /api/commands/exec POST branch in api/routes.py (single ~15-line addition next to the existing /api/commands GET branch).
  • static/commands.js additions: the executeAgentPluginCommand helper and the plugin-merge loop in getMatchingCommands. Drop the cmdGoal deletion and the _agentCommandCacheReady rename if it conflicts.
  • static/messages.js additions: the _agentCmd.category==='Plugin' branch in send(). Drop the deletions of _resetAssistantSegment, interim_assistant, goal, goal_continue, and the bottom-follow done logic.
  • tests/test_cli_only_slash_commands.py adjustments (3 lines).

That should land at ~150 net additions, 0 deletions, against 0b7e1e60, and CI will run.

Happy to look again once the diff is rebased.

@plerohellec
Copy link
Copy Markdown
Contributor Author

I really don't think it should be merged as is, it's a pure AI PR and I'm unable to say if it's any good. I only created the PR because somebody asked to see it in the issue. The issue #1935 is really the most important thing here, the PR is only here for illustration purposes of a working fix.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Understood — appreciate the transparency that this is illustrative for #1935 rather than a merge candidate. That actually shifts what would be useful next.

The diff against master is large — api/routes.py shrinks by ~540 lines while api/commands.py grows by ~64, static/commands.js and static/messages.js are restructured, and the test deletions in tests/test_cli_only_slash_commands.py mean some existing coverage gets dropped without replacement. That's the kind of rewrite that's hard to land as-is even when it works locally — the surface area is too broad to land "by inspection" without a code review against each restructured callsite, and the GPT-5.4-authored commits don't preserve the original PR-author attribution conventions this repo uses (Co-authored-by trailers, CHANGELOG credit lines).

For #1935 specifically — the actual contract gap is much narrower than this diff suggests. The current api/commands.py:get_commands() filters out anything not in the WebUI-known list rather than reading from the agent's hermes_cli/commands.py:COMMAND_REGISTRY and plugin-registered slash commands. That's the one-ish-screen change worth landing standalone. I'd suggest:

  1. Keep this PR open as illustrative reference (or convert to draft) so Slash command created in hermes-agent plugin can't be used in webui #1935 has a concrete "this does work locally" data point.
  2. File a focused PR that only touches api/commands.py to expose plugin-defined slash commands, with the agent contract documented (which hermes_cli symbols WebUI is allowed to import, what the response shape is). The cross-repo check for that lives in ~/.hermes/hermes-agent/hermes_cli/plugins.py:1255-1322register_plugin_command() is what builds the runtime registry, and the equivalent introspection on the agent side is list_registered_commands() / similar.
  3. Defer the routes.pycommands.jsmessages.js restructure to a separate PR that's scoped, reviewed, and has its own test coverage. That work probably belongs in the refactor(routes): split 8584-line api/routes.py into per-domain modules #1907 umbrella (route-splitting) rather than as a side-effect of a slash-command fix.

The narrow fix is a reasonable contributor PR if someone wants to take it. The wide rewrite is a maintainer-led refactor.

If the broader rewrite shape here is something maintainers actively want, I'd defer to them — but in its current form I don't think it's reviewable as-is, and the inability to certify "this is correct" is a fair reason not to merge.

@plerohellec plerohellec closed this May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants