fix(cli): make /tools slash commands ANSI-safe and idempotent#9205
Open
XiaoXiao0221 wants to merge 1 commit into
Open
fix(cli): make /tools slash commands ANSI-safe and idempotent#9205XiaoXiao0221 wants to merge 1 commit into
XiaoXiao0221 wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes two related interactive
/toolsslash-command bugs in the prompt_toolkit TUI:/tools list,/tools enable, and/tools disablecould print raw ANSI fragments like?[32minstead of rendered colors because the interactive path was not consistently routing output through the prompt_toolkit renderer./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 enableand/tools disable.Related Issue
Fixes #9017
No separate issue was filed for the repeated no-op reset path.
Type of Change
Changes Made
cli.py/tools listoutput through_cprint/tools enable|disableresult lines through_cprintNo session reset needed.for no-op repeated toggleshermes_cli/tools_config.pyline_printerplumbing so interactive TUI callers can render ANSI correctly without changing standalone CLI behaviorNo changes (already enabled/disabled): ...for repeated no-op operationstests/cli/test_cli_tools_command.py_cprintin unit tests so prompt_toolkit output is exercised safely on Windows test runnerstests/hermes_cli/test_tools_disable_enable.pyNo changes (...)output for idempotent callsHow to Test
/tools listand verify the output is clean text with rendered status markers, not raw ANSI fragments like?[32m./tools disable code_executiontwice and verify the second run saysNo changes (already disabled): code_executionand does not start a new session./tools enable code_executiontwice and verify the second run saysNo changes (already enabled): code_executionand does not start a new session.hermes tools list --platform cliand verify the standalone CLI path still prints correctly.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passNotes 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.pypytest tests/cli/test_cli_tools_command.py tests/hermes_cli/test_tools_disable_enable.py -qpytest tests/ -qandpytest tests/ -q -n 1, but both were terminated by the OS with exit code137on this Windows machine before completion.Documentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs
Interactive Windows CLI after the fix: