Skip to content

fix(cli): make classic CLI /indicator command branch aware - #34133

Open
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/cli-indicator-command-dispatch
Open

fix(cli): make classic CLI /indicator command branch aware#34133
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/cli-indicator-command-dispatch

Conversation

@Dusk1e

@Dusk1e Dusk1e commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a classic CLI dispatch regression by binding the registered /indicator slash command to a dedicated execution branch within the main handler pipeline, ensuring user configuration state synchronization matches the TUI paradigm.

Why

A parity gap existed within the command terminal routing framework. While /indicator was actively present inside the autocomplete registries and /help manifest definitions, the legacy command parser within cli.py lacked a matching execution leg. Running the command triggered an erroneous fallback outputting Unknown command to the operator. This patch plucks the missing route out of limbo, routes parameters directly into a proper status handler, and aligns state mutations cleanly with display.tui_status_indicator configuration schemas.

Scope of Changes

  • cli.py: Inserted explicit /indicator parsing steps into HermesCLI.process_command() and implemented a lightweight terminal indicator configuration interface.
  • tests/cli/test_cli_init.py: Added regression logic ensuring that /indicator payloads resolve straight to designated command blocks instead of getting dropped into structural missing-route blocks.
  • tests/cli/test_indicator_command.py: Introduced target verification sweeps asserting clear output execution rules across non-argument, valid mutation, and illegal value configurations.

Verified Test Suites

Targeted command-parsing structures and state change validation blocks finished with absolute success:

  • tests/cli/test_cli_init.py
  • tests/cli/test_indicator_command.py
  • tests/test_tui_gateway_server.py
Targeted Terminal Route Asserts: 45 passed, 0 failed
Interface Telemetry Controls: 7 passed, 179 deselected on criteria hooks

@Dusk1e
Dusk1e marked this pull request as draft May 28, 2026 23:11
@Dusk1e
Dusk1e marked this pull request as ready for review May 28, 2026 23:11
@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 duplicate This issue or pull request already exists labels May 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of the /indicator wiring cluster — canonical issue #22960, previously attempted in closed PRs #22962, #23289, #23037, #27627. The /sessions half was merged via #25986; /indicator still needs a fix. Note: /sessions was already merged, so if this PR also wires /sessions, that branch is redundant.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the registry-to-dispatch gap: the bug remains on current main. hermes_cli/commands.py:163 registers /indicator, while cli.py:8915-8921 goes from the /skin branch directly to /voice and ultimately to the unknown-command fallback at cli.py:9083.

Problems

  • The proposed handler location is stale. Current HermesCLI inherits CLICommandsMixin (cli.py:3677), and the extraction boundary explicitly puts _handle_*_command methods in hermes_cli/cli_commands_mixin.py:1-5; /skin now lives there at hermes_cli/cli_commands_mixin.py:2250. Adding the new handler back to cli.py would conflict with that organization.

Suggested changes

  • Keep the dispatch branch in cli.py, but place _handle_indicator_command and its validation beside the other command handlers in hermes_cli/cli_commands_mixin.py.
  • Preserve the PR's dispatch and config-validation tests. The target config path is correct: tui_gateway/server.py:10700-10712 already validates these four styles and writes display.tui_status_indicator.

Automated hermes-sweeper review.

Comment thread cli.py
@@ -9535,6 +9547,40 @@ def _handle_skin_command(self, cmd: str):
if self._apply_tui_skin_style():
print(" Prompt + TUI colors updated.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main extracted _handle_*_command methods into hermes_cli/cli_commands_mixin.py (_handle_skin_command is now there at line 2250). Please move this handler and its local validation support into that mixin; keep only the dispatch branch in cli.py.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 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 duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants