Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -9607,7 +9607,15 @@ def cmd_tools(args):
)
mcp_add_p.add_argument("name", help="Server name (used as config key)")
mcp_add_p.add_argument("--url", help="HTTP/SSE endpoint URL")
mcp_add_p.add_argument("--command", help="Stdio command (e.g. npx)")
# dest="mcp_command" so this flag does not clobber the top-level
# subparser's args.command attribute, which the dispatcher reads to
# route to cmd_mcp. Without an explicit dest, argparse derives
# dest="command" from the flag name and sets it to None when the
# flag is omitted, causing `hermes mcp add ...` to fall through to
# interactive chat.
mcp_add_p.add_argument(
"--command", dest="mcp_command", help="Stdio command (e.g. npx)"
)
mcp_add_p.add_argument(
"--args", nargs="*", default=[], help="Arguments for stdio command"
)
Expand Down
5 changes: 4 additions & 1 deletion hermes_cli/mcp_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,10 @@ def cmd_mcp_add(args):
"""Add a new MCP server with discovery-first tool selection."""
name = args.name
url = getattr(args, "url", None)
command = getattr(args, "command", None)
# Read from `mcp_command` (set by --command via explicit dest) — see
# mcp_add_p.add_argument("--command", dest="mcp_command", ...) in
# hermes_cli/main.py for why the dest is renamed.
command = getattr(args, "mcp_command", None)
cmd_args = getattr(args, "args", None) or []
auth_type = getattr(args, "auth", None)
preset_name = getattr(args, "preset", None)
Expand Down
87 changes: 87 additions & 0 deletions tests/hermes_cli/test_mcp_add_command_dest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
"""Regression test: ``hermes mcp add --command`` must not clobber the
top-level ``args.command`` subparser dest.

The top-level argparse parser uses ``dest="command"`` for its subparsers
(``hermes_cli/_parser.py``). The dispatcher in ``hermes_cli/main.py``
reads ``args.command`` to decide which command to run; if it is ``None``
it falls through to interactive chat.

The ``mcp add`` subparser exposes a ``--command`` flag (the stdio command
for an MCP server, e.g. ``npx``). Without an explicit ``dest=``, argparse
derives the dest from the flag name and writes ``args.command = None``
when the flag is omitted, overwriting the top-level ``"mcp"`` value. As a
result, ``hermes mcp add foo --url ...`` silently launches chat instead
of registering an MCP server.

The fix: declare the flag with ``dest="mcp_command"``. The CLI flag name
is unchanged; only the in-memory attribute moves.

We replicate the relevant parser shape here rather than importing the
real builder, mirroring ``test_argparse_flag_propagation.py`` and
``test_subparser_routing_fallback.py``.
"""

import argparse


def _build_parser():
"""Minimal replica of the slice of the hermes parser that exhibits
the bug: top-level subparsers (dest="command") and ``mcp add`` with
its ``--command`` flag.
"""
parser = argparse.ArgumentParser(prog="hermes")
subparsers = parser.add_subparsers(dest="command")

subparsers.add_parser("chat")

mcp_p = subparsers.add_parser("mcp")
mcp_sub = mcp_p.add_subparsers(dest="mcp_action")

mcp_add = mcp_sub.add_parser("add")
mcp_add.add_argument("name")
mcp_add.add_argument("--url")
mcp_add.add_argument("--command", dest="mcp_command")

return parser


class TestMcpAddCommandDest:
def test_url_invocation_preserves_top_level_command(self):
"""`hermes mcp add foo --url ...` must keep args.command == "mcp".

Before the dest fix this was clobbered to None, sending the
dispatcher into the chat fallback.
"""
parser = _build_parser()
args = parser.parse_args(
["mcp", "add", "foo", "--url", "https://example.com/mcp"]
)

assert args.command == "mcp"
assert args.mcp_action == "add"
assert args.name == "foo"
assert args.url == "https://example.com/mcp"
assert args.mcp_command is None

def test_command_flag_writes_to_mcp_command_dest(self):
"""`--command npx` must populate args.mcp_command, not args.command."""
parser = _build_parser()
args = parser.parse_args(
["mcp", "add", "github", "--command", "npx"]
)

assert args.command == "mcp"
assert args.mcp_command == "npx"

def test_bare_mcp_add_does_not_clobber_command(self):
"""Even without --url or --command, args.command stays "mcp".

Catches the regression at the parser layer regardless of which
transport flag the user passes.
"""
parser = _build_parser()
args = parser.parse_args(["mcp", "add", "foo"])

assert args.command == "mcp"
assert args.mcp_command is None
assert args.url is None
10 changes: 5 additions & 5 deletions tests/hermes_cli/test_mcp_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def _make_args(**kwargs):
defaults = {
"name": "test-server",
"url": None,
"command": None,
"mcp_command": None,
"args": None,
"auth": None,
"preset": None,
Expand Down Expand Up @@ -233,7 +233,7 @@ def mock_probe(name, config, **kw):

cmd_mcp_add(_make_args(
name="github",
command="npx",
mcp_command="npx",
args=["@mcp/github"],
))
out = capsys.readouterr().out
Expand Down Expand Up @@ -291,7 +291,7 @@ def mock_probe(name, config, **kw):

cmd_mcp_add(_make_args(
name="github",
command="npx",
mcp_command="npx",
args=["@mcp/github"],
env=["MY_API_KEY=secret123", "DEBUG=true"],
))
Expand All @@ -313,7 +313,7 @@ def test_add_stdio_server_rejects_invalid_env_name(self, capsys):

cmd_mcp_add(_make_args(
name="github",
command="npx",
mcp_command="npx",
args=["@mcp/github"],
env=["BAD-NAME=value"],
))
Expand Down Expand Up @@ -390,7 +390,7 @@ def mock_probe(name, config, **kw):
cmd_mcp_add(_make_args(
name="custom",
preset="testmcp",
command="uvx",
mcp_command="uvx",
args=["custom-server"],
))
out = capsys.readouterr().out
Expand Down