Skip to content

fix(cli): make /tools slash commands ANSI-safe and idempotent#9205

Open
XiaoXiao0221 wants to merge 1 commit into
NousResearch:mainfrom
XiaoXiao0221:fix/tools-slash-output-idempotence
Open

fix(cli): make /tools slash commands ANSI-safe and idempotent#9205
XiaoXiao0221 wants to merge 1 commit into
NousResearch:mainfrom
XiaoXiao0221:fix/tools-slash-output-idempotence

Conversation

@XiaoXiao0221

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes two related interactive /tools slash-command bugs in the prompt_toolkit TUI:

  1. /tools list, /tools enable, and /tools disable could print raw ANSI fragments like ?[32m instead of rendered colors because the interactive path was not consistently routing output through the prompt_toolkit renderer.
  2. Repeating /tools enable <tool> or /tools disable <tool> reported a false success and reset the session even when the tool was already in the requested state.

I searched existing open PRs before sending this. PRs #8630 and #9091 already cover the garbled ANSI subset, but this change also fixes the repeated no-op success/session-reset behavior for /tools enable and /tools disable.

Related Issue

Fixes #9017

No separate issue was filed for the repeated no-op reset path.

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

  • cli.py
    • route interactive /tools list output through _cprint
    • route interactive /tools enable|disable result lines through _cprint
    • only reset the session when the backend reports that tool state actually changed
    • show No session reset needed. for no-op repeated toggles
  • hermes_cli/tools_config.py
    • add line_printer plumbing so interactive TUI callers can render ANSI correctly without changing standalone CLI behavior
    • track which requested targets actually changed vs. were already enabled/disabled
    • return structured change metadata to the slash-command handler
    • emit No changes (already enabled/disabled): ... for repeated no-op operations
  • tests/cli/test_cli_tools_command.py
    • cover no-op slash-command behavior and ensure no session reset happens on repeated toggles
    • patch _cprint in unit tests so prompt_toolkit output is exercised safely on Windows test runners
  • tests/hermes_cli/test_tools_disable_enable.py
    • cover backend change metadata and the new No changes (...) output for idempotent calls

How to Test

  1. Start Hermes in the interactive CLI on Windows / PowerShell.
  2. Run /tools list and verify the output is clean text with rendered status markers, not raw ANSI fragments like ?[32m.
  3. Run /tools disable code_execution twice and verify the second run says No changes (already disabled): code_execution and does not start a new session.
  4. Run /tools enable code_execution twice and verify the second run says No changes (already enabled): code_execution and does not start a new session.
  5. Run hermes tools list --platform cli and verify the standalone CLI path still prints correctly.

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
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 + PowerShell 7

Notes on validation:

  • python -m py_compile cli.py hermes_cli/tools_config.py tests/cli/test_cli_tools_command.py tests/hermes_cli/test_tools_disable_enable.py
  • pytest tests/cli/test_cli_tools_command.py tests/hermes_cli/test_tools_disable_enable.py -q
  • Attempted full-suite runs with pytest tests/ -q and pytest tests/ -q -n 1, but both were terminated by the OS with exit code 137 on this Windows machine before completion.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • 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) per the compatibility guide — yes
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Interactive Windows CLI after the fix:

⚙️  /tools disable code_execution
Disabling code_execution...
✓ Disabled: code_execution
Session reset. New tool configuration is active.
(^_^)v New session started!

⚙️  /tools disable code_execution
Disabling code_execution...
No changes (already disabled): code_execution
No session reset needed.

⚙️  /tools enable code_execution
Enabling code_execution...
✓ Enabled: code_execution
Session reset. New tool configuration is active.
(^_^)v New session started!

⚙️  /tools enable code_execution
Enabling code_execution...
No changes (already enabled): code_execution
No session reset needed.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/tools Tool registry, model_tools, toolsets labels Apr 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Improve the display format of the /tools list command

2 participants