fix(cli): dispatch /agents inline while agent is running (#32477) - #32541
fix(cli): dispatch /agents inline while agent is running (#32477)#32541briandevans wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds inline (busy-path) dispatch for the classic CLI /agents command (and /tasks alias) so it runs immediately during an active agent loop instead of being queued behind process_loop and effectively doing nothing until the run completes.
Changes:
- Add
_should_handle_agents_command_inline()to detect when/agentsor/tasksshould bypass the queue while the agent is running - Update
handle_enter()to print and dispatch/agentsinline when busy - Add regression tests validating detector behavior and that
/agents//tasksdo not enqueue whenprocess_command()is invoked
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/cli/test_cli_agents_busy_path.py | Adds regression tests for inline detection/dispatch for /agents and /tasks while busy |
| cli.py | Implements the inline detector and triggers inline dispatch from the UI thread in handle_enter() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -7792,6 +7792,30 @@ def _should_handle_steer_command_inline(self, text: str, has_images: bool = Fals | |||
| except Exception: | |||
| return False | |||
|
|
|||
| def _should_handle_agents_command_inline(self, text: str, has_images: bool = False) -> bool: | |||
| if not text or has_images or not _looks_like_slash_command(text): | ||
| return False | ||
| if not getattr(self, "_agent_running", False): | ||
| return False | ||
| try: | ||
| from hermes_cli.commands import resolve_command | ||
| base = text.split(None, 1)[0].lower().lstrip('/') | ||
| cmd = resolve_command(base) | ||
| return bool(cmd and cmd.name == "agents") | ||
| except Exception: | ||
| return False |
| try: | ||
| from hermes_cli.commands import resolve_command | ||
| base = text.split(None, 1)[0].lower().lstrip('/') | ||
| cmd = resolve_command(base) |
| # makes it useless: the user sees "nothing happens" until the | ||
| # whole delegation chain finishes. See #32477. | ||
| if self._should_handle_agents_command_inline(text, has_images=has_images): | ||
| _cprint(f"\n⚙️ {text}") |
| """When the detector fires, /agents dispatch through process_command must | ||
| reach _handle_agents_command directly rather than being queued.""" | ||
|
|
||
| def test_process_command_routes_to_agents_handler(self): | ||
| """With _agent_running=True, /agents calls _handle_agents_command and | ||
| does NOT enqueue onto _pending_input.""" | ||
| cli = _make_cli() | ||
| cli._agent_running = True | ||
| cli._pending_input = MagicMock() | ||
| cli._handle_agents_command = MagicMock() | ||
|
|
||
| cli.process_command("/agents") | ||
|
|
||
| cli._handle_agents_command.assert_called_once() | ||
| cli._pending_input.put.assert_not_called() | ||
|
|
||
| def test_tasks_alias_routes_to_agents_handler(self): | ||
| """/tasks resolves to canonical 'agents' and dispatches the same.""" | ||
| cli = _make_cli() | ||
| cli._agent_running = True | ||
| cli._pending_input = MagicMock() | ||
| cli._handle_agents_command = MagicMock() | ||
|
|
||
| cli.process_command("/tasks") | ||
|
|
||
| cli._handle_agents_command.assert_called_once() |
| """When the detector fires, /agents dispatch through process_command must | ||
| reach _handle_agents_command directly rather than being queued.""" | ||
|
|
||
| def test_process_command_routes_to_agents_handler(self): | ||
| """With _agent_running=True, /agents calls _handle_agents_command and | ||
| does NOT enqueue onto _pending_input.""" | ||
| cli = _make_cli() | ||
| cli._agent_running = True | ||
| cli._pending_input = MagicMock() | ||
| cli._handle_agents_command = MagicMock() | ||
|
|
||
| cli.process_command("/agents") | ||
|
|
||
| cli._handle_agents_command.assert_called_once() | ||
| cli._pending_input.put.assert_not_called() | ||
|
|
||
| def test_tasks_alias_routes_to_agents_handler(self): | ||
| """/tasks resolves to canonical 'agents' and dispatches the same.""" | ||
| cli = _make_cli() | ||
| cli._agent_running = True | ||
| cli._pending_input = MagicMock() | ||
| cli._handle_agents_command = MagicMock() | ||
|
|
||
| cli.process_command("/tasks") | ||
|
|
||
| cli._handle_agents_command.assert_called_once() |
|
@copilot Addressed in commit 3b5eb124d:
The CI failure on |
…h#32477) Typing /agents (or its /tasks alias) during an active agent turn does nothing in classic CLI: the slash command goes through _pending_input, which process_loop only drains after self.chat() returns. By the time the queued command is pulled, the user has been staring at a quiet screen for the entire delegation chain — defeating the introspection command's only purpose. Mirror the inline-dispatch pattern PR NousResearch#25011 introduced for /steer: when the detector sees /agents while the agent is running, call process_command directly on the UI thread. _handle_agents_command is read-only (reads process_registry + a couple of CLI attrs) and _cprint already routes thread-unsafe output through prompt_toolkit's run_in_terminal, so UI-thread dispatch is safe. Fixes NousResearch#32477
Address Copilot inline review on NousResearch#32541: * Reject `/agents foo` / `/tasks something` on the busy path — `/agents` takes no args, so trailing tokens must fall through to the normal dispatch where `_handle_agents_command` can surface a usage error rather than silently dropping the argument. * Drop the `⚙️ {text}` echo from the busy-path block. Mirrors the existing `/steer` block which dispatches silently and lets `_handle_agents_command`'s own output land via `_cprint` / `run_in_terminal`. Avoids the screen-reader / terminal-font concern Copilot flagged. Adds a `test_ignores_agents_with_trailing_args` case covering the new detector constraint.
3b5eb12 to
b9d2f4a
Compare
|
Closing to focus the queue on security/file-safety work where civilian merges are landing. Happy to reopen if maintainers want this picked up. |
What does this PR do?
Typing
/agents(or its/tasksalias) during an active agent turn in the classic CLI does nothing — the user reports "the agents appear to be delegating properly, but attempting to monitor with /tasks or /agents does nothing". The slash command flows through_pending_input, whichprocess_looponly drains afterself.chat()returns; until then the queued command sits silently behind the running delegation chain. From the user's perspective the keystroke is lost during exactly the window the command exists for.Mirror the inline-dispatch pattern PR #25011 introduced for
/steer: detect/agentson the UI thread insidehandle_enterand callprocess_commanddirectly._handle_agents_commandis read-only (touchesprocess_registryplus a couple of CLI attrs) and_cprintalready routes thread-unsafe output throughprompt_toolkit'srun_in_terminal, so UI-thread dispatch is safe.Related Issue
Fixes #32477
Type of Change
Changes Made
cli.py— add_should_handle_agents_command_inline(parallel to_should_handle_steer_command_inline) and dispatch inline fromhandle_enterwhen the detector fires.tests/cli/test_cli_agents_busy_path.py— new regression suite mirroringtest_cli_steer_busy_path.py: detector returns True for/agentsand/taskswhile running, False when idle / non-slash / images attached / other slash commands;process_commandroutes to_handle_agents_commandwithout enqueueing on_pending_input.How to Test
hermes chatwith a delegating profile./agents(or/tasks). The active-agents + running-processes summary should print above the prompt immediately instead of disappearing into the queue.uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest tests/cli/test_cli_agents_busy_path.py tests/cli/test_cli_steer_busy_path.py -v— 15 passes.Checklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
Screenshots / Logs