mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-18 04:41:56 +00:00
fix(kanban): /kanban slash command emits argparse garbage instead of help
Closes #21794. `/kanban`, `/kanban help`, `/kanban --help`, and `/kanban <sub> -h` all returned broken output to the gateway and interactive CLI. Three underlying bugs in `hermes_cli.kanban.run_slash`: 1. argparse writes help to **stdout** but `run_slash` only captured stderr at parse time, so `-h` text was silently swallowed and replaced with the `(usage error: 0)` sentinel. 2. The wrapping parser used `prog="/"` and routed via a synthetic "_top → kanban" subparser, producing `usage: / kanban …` (stray space) and `usage: /kanban kanban …` (doubled token) in error text. 3. Bare `/kanban` and `/kanban help` dumped argparse's full ~3KB usage tree, which reads as visual garbage in a chat bubble. Fix: drive the kanban_parser directly (no double-wrap), rewrite prog strings on every leaf subparser, capture stdout AND stderr around parse_args, distinguish SystemExit(0) (help — return captured stdout) from SystemExit(2) (error — return single-line ⚠-prefixed message), and add an explicit chat-friendly short-help block returned for bare invocation and the help aliases (`help`, `--help`, `-h`, `?`). Added 5 regression tests covering bare invocation, every help alias, subcommand help, unknown action, and missing required arg. Affects every chat platform via gateway/run.py::_handle_kanban_command and the interactive CLI via cli.py::_handle_kanban_command. Co-Authored-By: Nagatha (Claude Opus 4.7) <noreply@anthropic.com>
This commit is contained in:
parent
3d2bfc502e
commit
d1fc748def
2 changed files with 118 additions and 23 deletions
|
|
@ -2136,6 +2136,29 @@ def _cmd_gc(args: argparse.Namespace) -> int:
|
||||||
# Slash-command entry point (used by /kanban from CLI and gateway)
|
# Slash-command entry point (used by /kanban from CLI and gateway)
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
_SLASH_KANBAN_HELP = """\
|
||||||
|
**/kanban** — manage the shared task board.
|
||||||
|
|
||||||
|
Common subcommands:
|
||||||
|
`list` (alias `ls`) List tasks on the current board
|
||||||
|
`show <id>` Task details + comments + events
|
||||||
|
`stats` Per-status / per-assignee counts
|
||||||
|
`create <title>…` Create a task (auto-subscribes you to events)
|
||||||
|
`comment <id> <msg>` Append a comment
|
||||||
|
`complete <id>…` Mark task(s) done
|
||||||
|
`block <id> [reason]` Mark blocked; `unblock <id>` to revive
|
||||||
|
`assign <id> <profile>` Reassign
|
||||||
|
`boards list` Show all boards
|
||||||
|
`assignees` Known profiles + counts
|
||||||
|
`context <id>` Full worker-context dump
|
||||||
|
`runs <id>` Attempt history
|
||||||
|
`log <id>` Worker log
|
||||||
|
|
||||||
|
Run `/kanban <subcommand> -h` for arguments. \
|
||||||
|
Read-only commands are safe while an agent is running.\
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
def run_slash(rest: str) -> str:
|
def run_slash(rest: str) -> str:
|
||||||
"""Execute a ``/kanban …`` string and return captured stdout/stderr.
|
"""Execute a ``/kanban …`` string and return captured stdout/stderr.
|
||||||
|
|
||||||
|
|
@ -2148,26 +2171,47 @@ def run_slash(rest: str) -> str:
|
||||||
|
|
||||||
tokens = shlex.split(rest) if rest and rest.strip() else []
|
tokens = shlex.split(rest) if rest and rest.strip() else []
|
||||||
|
|
||||||
parser = argparse.ArgumentParser(prog="/kanban", add_help=False)
|
# Bare ``/kanban`` or ``/kanban help`` / ``--help`` / ``-h`` / ``?``:
|
||||||
parser.exit_on_error = False # type: ignore[attr-defined]
|
# show the curated short-help block instead of dumping argparse's full
|
||||||
sub = parser.add_subparsers(dest="kanban_action")
|
# usage tree (which is enormous and reads as garbage in a chat
|
||||||
# Reuse the argparse builder -- call it with a throwaway parent
|
# bubble). Per-subcommand help still works via ``/kanban foo -h``.
|
||||||
# subparsers via a wrapping top-level parser.
|
if not tokens or tokens[0] in {"help", "--help", "-h", "?"}:
|
||||||
wrap = argparse.ArgumentParser(prog="/", add_help=False)
|
return _SLASH_KANBAN_HELP
|
||||||
wrap.exit_on_error = False # type: ignore[attr-defined]
|
|
||||||
wrap_sub = wrap.add_subparsers(dest="_top")
|
# Single argparse tree rooted at "/kanban". build_parser() expects a
|
||||||
build_parser(wrap_sub)
|
# subparsers action to attach to, so build a throwaway one and pull
|
||||||
|
# the kanban_parser back out — then drive it directly so usage/error
|
||||||
|
# text reads as ``/kanban`` (not ``/kanban-wrap kanban``).
|
||||||
|
_wrap = argparse.ArgumentParser(prog="/kanban-wrap", add_help=False)
|
||||||
|
_wrap.exit_on_error = False # type: ignore[attr-defined]
|
||||||
|
_top_sub = _wrap.add_subparsers(dest="_top")
|
||||||
|
kanban_parser = build_parser(_top_sub)
|
||||||
|
kanban_parser.prog = "/kanban"
|
||||||
|
kanban_parser.exit_on_error = False # type: ignore[attr-defined]
|
||||||
|
for _action in kanban_parser._actions:
|
||||||
|
if isinstance(_action, argparse._SubParsersAction):
|
||||||
|
for _name, _choice in _action.choices.items():
|
||||||
|
_choice.prog = f"/kanban {_name}"
|
||||||
|
_choice.exit_on_error = False # type: ignore[attr-defined]
|
||||||
|
|
||||||
buf_out = io.StringIO()
|
buf_out = io.StringIO()
|
||||||
buf_err = io.StringIO()
|
buf_err = io.StringIO()
|
||||||
|
# ``-h`` / ``--help`` makes argparse print to stdout and SystemExit(0).
|
||||||
|
# Capture both streams so neither the help text nor the error text
|
||||||
|
# bypasses our buffer.
|
||||||
try:
|
try:
|
||||||
# Prepend the "kanban" token so our top-level subparser routes here.
|
with contextlib.redirect_stdout(buf_out), contextlib.redirect_stderr(buf_err):
|
||||||
argv = ["kanban", *tokens] if tokens else ["kanban"]
|
args = kanban_parser.parse_args(tokens)
|
||||||
args = wrap.parse_args(argv)
|
|
||||||
except SystemExit as exc:
|
except SystemExit as exc:
|
||||||
return f"(usage error: {exc})"
|
out = buf_out.getvalue().rstrip()
|
||||||
|
err = buf_err.getvalue().rstrip()
|
||||||
|
# Help dump (exit 0) → return the captured help text directly.
|
||||||
|
if exc.code in (0, None) and out:
|
||||||
|
return out
|
||||||
|
body = err or out
|
||||||
|
return f"⚠ /kanban usage error\n{body}" if body else "⚠ /kanban usage error"
|
||||||
except argparse.ArgumentError as exc:
|
except argparse.ArgumentError as exc:
|
||||||
return f"(usage error: {exc})"
|
return f"⚠ /kanban usage error: {exc}"
|
||||||
|
|
||||||
with contextlib.redirect_stdout(buf_out), contextlib.redirect_stderr(buf_err):
|
with contextlib.redirect_stdout(buf_out), contextlib.redirect_stderr(buf_err):
|
||||||
try:
|
try:
|
||||||
|
|
|
||||||
|
|
@ -331,13 +331,64 @@ def test_run_slash_specify_end_to_end(kanban_home, monkeypatch):
|
||||||
|
|
||||||
|
|
||||||
def test_run_slash_specify_help_is_reachable(kanban_home):
|
def test_run_slash_specify_help_is_reachable(kanban_home):
|
||||||
"""`--help` on a subcommand is handled by argparse itself — it prints
|
"""`-h`/`--help` on a subcommand returns the actual help text — see
|
||||||
to the process stdout and raises SystemExit before run_slash's output
|
issue #21794. argparse writes help to stdout and exits 0; run_slash
|
||||||
redirection is installed, so the returned string is the usage-error
|
must capture both streams and treat exit 0 as success, not error."""
|
||||||
sentinel. All we're asserting here is that the subcommand is
|
|
||||||
registered (no "unknown action" error) — the shape of the help text
|
|
||||||
is covered by the direct argparse tests in test_kanban_specify.py."""
|
|
||||||
out = kc.run_slash("specify --help")
|
out = kc.run_slash("specify --help")
|
||||||
# Either the usage-error sentinel (stdout swallowed by argparse) or
|
assert "specify" in out.lower()
|
||||||
# a real help rendering — both mean the subcommand exists.
|
# Help dump should NOT come back wrapped as a usage error.
|
||||||
assert "usage error" in out.lower() or "specify" in out.lower()
|
assert not out.startswith("⚠")
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# /kanban help / no-args / unknown-action UX (issue #21794)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_run_slash_bare_returns_curated_help(kanban_home):
|
||||||
|
"""Bare `/kanban` returns the curated short-help block — not a 5KB
|
||||||
|
argparse usage dump."""
|
||||||
|
out = kc.run_slash("")
|
||||||
|
assert "/kanban" in out
|
||||||
|
assert "list" in out
|
||||||
|
assert "show" in out
|
||||||
|
# Sanity: should be a chat-friendly size, not the raw usage tree.
|
||||||
|
assert len(out) < 2000
|
||||||
|
# Shouldn't surface argparse's usage-error sentinel.
|
||||||
|
assert "usage error" not in out.lower()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("alias", ["help", "--help", "-h", "?"])
|
||||||
|
def test_run_slash_help_aliases_match_bare(kanban_home, alias):
|
||||||
|
"""Every documented help alias produces the same curated output."""
|
||||||
|
bare = kc.run_slash("")
|
||||||
|
out = kc.run_slash(alias)
|
||||||
|
assert out == bare
|
||||||
|
|
||||||
|
|
||||||
|
def test_run_slash_subcommand_help_returns_help_text(kanban_home):
|
||||||
|
"""`/kanban show -h` returns the actual subcommand help, not a
|
||||||
|
fake `(usage error: 0)` sentinel."""
|
||||||
|
out = kc.run_slash("show -h")
|
||||||
|
assert "task_id" in out
|
||||||
|
assert "/kanban show" in out
|
||||||
|
assert not out.startswith("⚠")
|
||||||
|
|
||||||
|
|
||||||
|
def test_run_slash_unknown_action_friendly_error(kanban_home):
|
||||||
|
"""Unknown subcommand surfaces a single-line usage error prefixed
|
||||||
|
with our marker — no `(usage error: 2)` wrapping, no doubled
|
||||||
|
`kanban kanban` prog string."""
|
||||||
|
out = kc.run_slash("frobnicate")
|
||||||
|
assert "/kanban" in out
|
||||||
|
assert "frobnicate" in out
|
||||||
|
assert "/kanban-wrap" not in out
|
||||||
|
assert "/kanban kanban" not in out
|
||||||
|
assert "(usage error: " not in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_run_slash_missing_required_arg_friendly_error(kanban_home):
|
||||||
|
"""Missing positional argument shows the subcommand-scoped usage
|
||||||
|
line, not the top-level kanban tree."""
|
||||||
|
out = kc.run_slash("show")
|
||||||
|
assert "/kanban show" in out
|
||||||
|
assert "task_id" in out
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue