fix(cli): refuse an unbalanced-quote slash command instead of killing the session (supersedes #43503) by briandevans · Pull Request #76887 · NousResearch/hermes-agent

briandevans · 2026-08-02T15:48:05Z

What does this PR do?

Supersedes #43503. The diagnosis there is @ly-wang19's and it is correct: shlex.split raises ValueError: No closing quotation on an unbalanced quote, the interactive REPL dispatch wraps process_command in except KeyboardInterrupt only (cli.py:17307-17322), and the comment on that guard already names the consequence — anything else "unwinds to the outer prompt_toolkit loop and the session dies." One stray " ends the session and the conversation with it. That PR has had no push since the 2026-07-14 review, so this carries it forward with the two changes that review asked for.

Delta 1 — refuse instead of degrade (the blocking review point). #43503 catches ValueError and falls back to cmd.split(). That keeps the session alive but lets the malformed line execute, which is exactly what the review flagged: "whitespace fallback can execute malformed input … /cron add 30m "partial can therefore create a job because 30m is valid", and "On ValueError, print an invalid-quoting message and return from both handlers; do not call cmd.split() or delegate the malformed command." Verified on today's main: a naive split gives ['/cron', 'add', '30m', '"partial'], _handle_cron_command takes positionals[0] as the schedule (cli_commands_mixin.py:1596) and calls _cron_api(action="create", …) at :1602, so a half-typed line silently creates a real scheduled job. /curator likewise hands '"unterminated' straight to hermes_cli.curator.cli_main. This PR returns before the API call and before the delegation.

Delta 2 — the third live site. #43503 covers 2 of 3. _handle_journey_command (cli_commands_mixin.py:485) has the identical unguarded shlex.split, sitting inside a try: whose only handler is except SystemExit:, dispatched bare from cli.py:10209. It was not in the handler audit on that PR — /journey started using shlex after it.

Related Issue

No issue is filed for this; the premise is #43503's and is re-verified against current main below.

Type of Change

🐛 Bug fix (non-breaking change that fixes an issue)
✨ New feature (non-breaking change that adds functionality)
🔒 Security fix
📝 Documentation update
✅ Tests (adding or improving test coverage)
♻️ Refactor (no behavior change)
🎯 New skill (bundled or hub)

Changes Made

hermes_cli/cli_commands_mixin.py — new _tokenize_slash_command(text, *, label, example) helper: returns tokens, or prints a quoting hint and returns None. One helper so the three sites cannot drift apart again.
hermes_cli/cli_commands_mixin.py:485 _handle_journey_command — refuse before parser.parse_args; parse_args keeps its own separate except SystemExit.
hermes_cli/cli_commands_mixin.py:1487 _handle_cron_command — refuse before subcommand = tokens[1].lower() and before any _cron_api(...).
hermes_cli/cli_commands_mixin.py:1719 _handle_curator_command — refuse before the from hermes_cli.curator import cli_main delegation; the bare-/curator → ["status"] default is preserved.
The now-dead per-handler import shlex lines are removed from all three handlers.
The hint is emitted via cli._cli_visible_print, not bare print: patch_stdout swallows bare print while the prompt_toolkit Application owns the terminal (cli.py:3200-3217), which is exactly the situation this guard exists for.
tests/cli/test_cli_slash_quote_refusal.py — new, 11 tests.

Sibling-site sweep

grep -rn 'shlex\.split' --include='*.py' . over production code, then filtered to user-typed input reaching a handler on the CLI REPL slash dispatch. Every site is accounted for:

Fixed — unguarded, on the slash-dispatch path, ValueError kills the session:

site	handler	slash	dispatch
`hermes_cli/cli_commands_mixin.py:485`	`_handle_journey_command`	`/journey`	`cli.py:10209`
`hermes_cli/cli_commands_mixin.py:1487`	`_handle_cron_command`	`/cron`	`cli.py:10040`
`hermes_cli/cli_commands_mixin.py:1719`	`_handle_curator_command`	`/curator`	`cli.py:10046`

Deliberately excluded, with reasons:

site	why it is not the same root cause
`:159` `/diff`, `:658` `/tools`, `:1670` `/suggestions`, `:1694` `/blueprint`	already `try/except ValueError`. They use the naive-split fallback, but the degraded parse there is not effectful — no job is created, nothing is delegated. Changing them would alter commands that work today, which is a separate behaviour question and is left out of this PR on purpose.
`:2607` `_compose_in_editor`	already inside `try/except Exception`, and it splits the `$EDITOR` config value, not typed input.
`hermes_cli/kanban.py:3171`	its caller `_handle_kanban_command` (`:1747-1750`) wraps `run_slash` in `except Exception`.
`hermes_cli/blueprint_cmd.py:270`, `hermes_cli/mcp_security.py:96`	already `try/except ValueError`.
`hermes_cli/console_engine.py:113`	raises a typed `ConsoleCommandError`, caught by its own REPL at `:527` / `:1136`. Different surface.
`hermes_cli/session_listing.py:23`	`parse_session_listing_args` is only reached from `gateway/slash_commands.py:4521`; `cli.py:7844` imports `query_session_listing` instead. Different dispatch, different blast radius.
`hermes_cli/main.py:6957`	parses the `desktop.electron_flags` config value, not typed input.
`agent/`, `gateway/`, `tools/`, `plugins/` sites	not on the CLI REPL slash-dispatch path.

How to Test

uv run --with pytest --with pytest-asyncio python3 -m pytest tests/cli/test_cli_slash_quote_refusal.py -v

Manually: start hermes, type /cron add 30m "partial and press enter. Before this change the session exits. After it, you get a hint and the prompt is still there — and /cron list shows no new job.

Fails-before / passes-after, all three states

A — clean main (no guard at all): 6 failed, 4 passed.

FAILED test_cron_unbalanced_quote_does_not_create_a_job     - ValueError: No closing quotation
FAILED test_curator_unbalanced_quote_does_not_delegate      - ValueError: No closing quotation
FAILED test_journey_unbalanced_quote_does_not_dispatch      - ValueError: No closing quotation
FAILED test_slash_handlers_survive_unbalanced_quote[_handle_journey_command-...]
FAILED test_slash_handlers_survive_unbalanced_quote[_handle_cron_command-...]
FAILED test_slash_handlers_survive_unbalanced_quote[_handle_curator_command-...]

B — main + #43503's cmd.split() fallback: 4 failed, 6 passed. The session survives, but the malformed input runs — this is the review's objection, reproduced:

E  AssertionError: Expected 'cronjob' to not have been called. Called 1 times.
E  Calls: [call(action='create', schedule='30m', prompt='"partial', name=None,
E          deliver=None, repeat=None, skills=None)].

E  AssertionError: Expected 'cli_main' to not have been called. Called 1 times.
E  Calls: [call(['"unterminated'])].

plus /journey still dying with ValueError: No closing quotation in 2 tests, since that site is not covered there.

C — this PR: 11 passed (10 at the time of the run below, plus test_refusal_hint_uses_the_prompt_toolkit_safe_printer added in a25032bad; that one fails if the helper regresses to a bare print).

tests/cli/test_cli_slash_quote_refusal.py::test_cron_unbalanced_quote_does_not_create_a_job PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_cron_balanced_quotes_still_group_schedule_and_prompt PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_curator_unbalanced_quote_does_not_delegate PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_curator_balanced_quotes_still_group_tokens PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_curator_bare_command_still_defaults_to_status PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_journey_unbalanced_quote_does_not_dispatch PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_journey_balanced_quotes_still_group_tokens PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_slash_handlers_survive_unbalanced_quote[_handle_journey_command-/journey delete "my entry] PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_slash_handlers_survive_unbalanced_quote[_handle_cron_command-/cron add 30m "partial] PASSED
tests/cli/test_cli_slash_quote_refusal.py::test_slash_handlers_survive_unbalanced_quote[_handle_curator_command-/curator "unterminated] PASSED

============================== 10 passed in 0.23s ==============================

Adjacent surface (every test file that imports the mixin or exercises /cron, /curator, /journey, /diff, /focus, /reasoning, curator, cron scheduling, write-approval, handoff relay) — 274 passed, serially, on this branch.

Checklist

Code

I've read the Contributing Guide
My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
I searched for existing PRs to make sure this isn't a duplicate
My PR contains only changes related to this fix/feature (no unrelated commits)
I've run pytest tests/ -q and all tests pass — ran the focused file plus the full adjacent surface (274 passed) and tests/cli + tests/hermes_cli; the remaining reds there reproduce identically on clean main and are unrelated to this change.
I've added tests for my changes (required for bug fixes, strongly encouraged for features)
I've tested on my platform: macOS 15 (arm64), Python 3.11.15

Documentation & Housekeeping

I've updated relevant documentation (README, docs/, docstrings) — the new helper carries the rationale in its docstring; no user-facing docs describe this path
I've updated cli-config.yaml.example if I added/changed config keys — N/A
I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
I've considered cross-platform impact (Windows, macOS) — shlex.split raises the same ValueError on every platform and no platform-specific behaviour is introduced; tested on macOS only
I've updated tool descriptions/schemas if I changed tool behavior — N/A

Related / Positioning

Supersedes fix(cli): don't crash the session on an unbalanced quote in /cron or /curator #43503 (@ly-wang19) — same premise, credited above; this changes the remedy from a naive-split fallback to a refusal, and adds the /journey site.
No overlap with the other open PRs on this file: feat(cron): keep scheduler state out of jobs.json (cron/runtime.db) #75833 (/cron edit semantics, @@ -1588), feat(cli): add clarification-first question mode #75969 (_handle_suggestions_command, @@ -1678), refactor(cli): extract session navigation into CLISessionNavigationMixin #75787 (extraction of 704-1245), feat(agent): add per-turn provider request budget #76458 (@@ -1911), fix(moa,desktop): sticky route integrity + interim double-bubble #76191 (@@ -1158), feat(cli): sessions list/search options, columns, pagination #75496 (854-1173), fix(goal): resume standing goal work immediately #75361 (@@ -2332), feat(cli): add width-safe streaming Markdown rendering #75326 (@@ -1970), fix(cli): honor skin colors in inline diffs #75216 (@@ -28 / -2565). None touches 485, 1487 or 1719.
fix(cli): align /cron slash handler with cron parser #24597 and fix(cli): use _cprint() for /cron command output #40536 mention these handlers but diff against the pre-refactor root cli.py, which no longer defines them (it does from hermes_cli.cli_commands_mixin import CLICommandsMixin).
The new test file is named test_cli_slash_quote_refusal.py rather than fix(cli): don't crash the session on an unbalanced quote in /cron or /curator #43503's test_cli_slash_unbalanced_quote.py so the two never collide as an add/add if both are ever applied.

… the session (supersedes NousResearch#43503) `shlex.split` raises `ValueError: No closing quotation` on an unbalanced quote. Three interactive-CLI slash handlers call it bare on the line the user typed: * hermes_cli/cli_commands_mixin.py:485 `_handle_journey_command` (inside a `try:` whose only handler is `except SystemExit:`) * hermes_cli/cli_commands_mixin.py:1487 `_handle_cron_command` * hermes_cli/cli_commands_mixin.py:1719 `_handle_curator_command` The REPL dispatch wraps `process_command` in `except KeyboardInterrupt` only (cli.py:17307-17322), and the comment there already names the consequence: anything else "unwinds to the outer prompt_toolkit loop and the session dies". So a single stray `"` ends the session and the conversation with it. Route all three through one `_tokenize_slash_command` helper that prints a quoting hint and returns None. This is deliberately not a `cmd.split()` fallback: a naive split keeps the session alive but lets the malformed line execute. `/cron add 30m "partial` splits to `['/cron', 'add', '30m', '"partial']`, `30m` validates as a schedule, and a scheduled job is silently created from input the user never finished typing; `/curator` likewise delegates `'"unterminated'` into `hermes_cli.curator.cli_main`. Refusing before the API call and before the delegation is what keeps the failure inert. The four already-guarded sites in this file (`/diff` :159, `/tools` :658, `/suggestions` :1670, `/blueprint` :1694) are left alone: they are not effectful on the degraded path, and changing them would alter commands that work today. Regression tests assert the cron API is never called and the curator entry point is never invoked for a malformed line, that `/journey` never dispatches, that no exception escapes any of the three handlers, and that correctly quoted commands still group their tokens.

Copilot

Pull request overview

This PR hardens the interactive CLI slash-command dispatch against unbalanced quotes by refusing to run malformed /journey, /cron, and /curator inputs (instead of letting a shlex.split() ValueError unwind and kill the prompt_toolkit session). It introduces a shared tokenizer helper and adds targeted regression coverage to ensure malformed input neither crashes the session nor executes partial commands.

Changes:

Add CLICommandsMixin._tokenize_slash_command() to centralize shlex.split() handling and refuse unbalanced-quote input.
Update /journey, /cron, and /curator handlers to use the helper and return early on refusal.
Add tests/cli/test_cli_slash_quote_refusal.py to cover refusal (no side effects) and survival (no exception escapes) across the three handlers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`hermes_cli/cli_commands_mixin.py`	Adds a shared slash tokenizer and routes `/journey`, `/cron`, `/curator` through refusal-on-unbalanced-quote behavior.
`tests/cli/test_cli_slash_quote_refusal.py`	Adds regression tests asserting malformed quoted input is refused, produces a hint, and does not execute side effects.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

briandevans · 2026-08-02T15:55:13Z

+        import shlex
+
+        try:
+            return shlex.split(text)
+        except ValueError as exc:
+            print(f"(._.) {label}: {exc}. Nothing was run.")
+            print(f"      Close the quote, e.g. {example}")
+            return None


Good catch — this was a real hole, not a style nit. The return was doing its job (no cron API call, no curator delegation), but patch_stdout would have eaten the hint in exactly the context the guard exists for, so the user would have seen their command do nothing at all with no explanation.

Fixed in a25032b:

_tokenize_slash_command now emits both lines through cli._cli_visible_print, which routes to _cprint while a prompt_toolkit Application is running and falls back to print otherwise (cli.py:3200-3217).

Added test_refusal_hint_uses_the_prompt_toolkit_safe_printer, which patches cli._cli_visible_print and asserts the refusal text goes through it. Verified it is not vacuous: reverting the helper to a bare print makes that test fail, restoring it makes it pass.

The docstring now records why, so the next edit does not quietly regress it.

Focused suite: 11 passed.

`patch_stdout` swallows bare `print` while the prompt_toolkit Application owns the terminal (`cli.py:3200-3217`), which is exactly the situation this guard exists for — so the refusal hint was silent in the interactive CLI even though the command was correctly refused. Route both lines through `cli._cli_visible_print`, which falls back to `print` when no Application is running. Adds a test that fails if the helper regresses to a bare `print`.

teknium1 · 2026-08-02T16:11:58Z

Thanks for carrying the earlier diagnosis forward and changing the unsafe whitespace fallback into a refusal.

I verified the premise on current main: unguarded shlex.split calls remain in hermes_cli/cli_commands_mixin.py:485, :1487, and :1719, while the interactive slash dispatch at cli.py:17307-17322 catches only KeyboardInterrupt. The /cron path reaches action="create" at hermes_cli/cli_commands_mixin.py:1566-1574, so returning before parsing is the appropriate inert failure mode. The prior prompt_toolkit visibility concern was addressed in a25032bad678 through cli._cli_visible_print.

Automated hermes-sweeper review.

GottZ

This was generated by AI during triage.

Summary

Two PRs address the unbalanced-quote crash in slash-command parsing. #43503 catches the parsing error for /cron and /curator but falls back to executable whitespace tokenization, whereas #76887 safely refuses malformed input, extends coverage to /journey, and tests both session survival and absence of side effects.

Related pull requests

#43503 duplicate — (+70/-2) — close as duplicate of #76887: the diff prevents the /cron and /curator parsing exception from escaping, but its cmd.split() fallback can execute malformed input such as /cron add 30m "partial and create a scheduled job. Despite the keep_open review on #43503, the documented /cron flow requires refusal before dispatch; #76887 implements that correction.
#76887 related — (+216/-7) — keep open with a salvage path: retain the shared refusal tokenizer, early returns for malformed /journey, /cron, and /curator commands, no-dispatch/no-cron-side-effect tests, and the prompt_toolkit-visible hint added in a25032bad678. This aligns with the maintainer-bot keep_open verdict while supplying the safe replacement for #43503.

Duplicates

#76887 supersedes #43503 for the same unbalanced-quote failure; #43503 is the unsafe whitespace-fallback variant and can be closed as a duplicate of #76887.

Suggested consolidation

Close #43503 as duplicate of #76887. Keep #76887 open with the concrete salvage path already present in its diff: refuse malformed quoting before dispatch, preserve the shared three-handler tokenizer and no-side-effect regression coverage, and retain the prompt_toolkit-visible feedback from a25032bad678.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup43503 ["PRs duplicating each other"]
        P43503["PR #43503 (open)"]
        P76887["PR #76887 (open)"]
    end
    class P43503 open
    class P76887 open
    class P76887 target
    click P43503 "https://github.com/NousResearch/hermes-agent/pull/43503"
    click P76887 "https://github.com/NousResearch/hermes-agent/pull/76887"

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 13 kB of issue/PR text, 7 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Copilot AI review requested due to automatic review settings August 2, 2026 15:48

Copilot started reviewing on behalf of briandevans August 2, 2026 15:48 View session

Copilot AI reviewed Aug 2, 2026

View reviewed changes

teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Aug 2, 2026

alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists labels Aug 2, 2026

GottZ mentioned this pull request Aug 3, 2026

fix(cli): don't crash the session on an unbalanced quote in /cron or /curator #43503

Open

GottZ reviewed Aug 3, 2026

View reviewed changes

alt-glitch mentioned this pull request Aug 25, 2026

fix(gateway): /kanban must not raise on unbalanced quotes in args #94571

Open

cervantesh mentioned this pull request Sep 18, 2026

[Wave] "Supersedes #N" watch-list — 137 rows, re-sliced by what is actionable now (corrected 2026-09-18) #114510

Open

Navigation Menu

fix(cli): refuse an unbalanced-quote slash command instead of killing the session (supersedes #43503) - #76887

fix(cli): refuse an unbalanced-quote slash command instead of killing the session (supersedes #43503)#76887
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/cli-slash-quote-refusal-43503

briandevans commented Aug 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

briandevans Aug 2, 2026

Uh oh!

teknium1 commented Aug 2, 2026

Uh oh!

GottZ left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

briandevans commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Related Issue

Type of Change

Changes Made

Sibling-site sweep

How to Test

Fails-before / passes-after, all three states

Checklist

Code

Documentation & Housekeeping

Related / Positioning

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

briandevans Aug 2, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 commented Aug 2, 2026

Uh oh!

GottZ left a comment

Choose a reason for hiding this comment

Summary

Related pull requests

Duplicates

Suggested consolidation

Complex graph

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

briandevans commented Aug 2, 2026 •

edited

Loading