Conversation
WalkthroughThis pull request introduces tmux-based SSH session management to the Aegis CLI. A new ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
praetorian_cli/ui/aegis/commands/ssh.py (1)
75-113:⚠️ Potential issue | 🟠 Major
-l USERis still being rewritten as-L USER.This path depends on
praetorian_cli/handlers/ssh_utils.py:34-40, and that parser currently treats lowercase-laslocal_forward. Sossh -l rootbecomesssh -L root ...instead of selecting the SSH user.Suggested fix in
praetorian_cli/handlers/ssh_utils.py- if arg in ['-L', '-l', '--local-forward']: + if arg in ['-L', '--local-forward']: if i + 1 >= len(args): self._print_error("Error: -L requires a port forwarding specification") self._print_error("Example: ssh -L 8080:localhost:80", dim=True) return None options['local_forward'].append(args[i + 1]) i += 2 @@ - elif arg in ['-u', '-U', '--user']: + elif arg in ['-l', '-u', '-U', '--user']: if i + 1 >= len(args): self._print_error("Error: -u requires a username") self._print_error("Example: ssh -u root", dim=True) return None options['user'] = args[i + 1]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@praetorian_cli/ui/aegis/commands/ssh.py` around lines 75 - 113, The parser is treating lowercase "-l" as a local-forward flag so parse_ssh_args/SSHArgumentParser is turning "-l USER" into a local_forward entry which later gets emitted as "-L USER"; update the parsing rules in the SSHArgumentParser/parse_ssh_args implementation (and the underlying ssh_utils parser) to reserve "-l" for the SSH login user (populate parsed_options['user'] or similar) and only map "-L"/"--local" to parsed_options['local_forward']; after that ensure the code that reconstructs options uses parsed_options['user'] to emit "-l USER" (not as a forward) and continues to emit "-L" for entries in parsed_options['local_forward'] so ssh -l root becomes "-l root" instead of "-L root".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@praetorian_cli/ui/aegis/commands/ssh.py`:
- Around line 49-56: The SSH command currently routes 'list' and 'kill' before
the selected-agent guard but still rejects 'ssh help' / 'ssh --help'; add an
early branch like the existing ones that checks if len(args) and args[0].lower()
in ('help', '--help', '-h') and call the SSH help printer (create or reuse a
helper such as _show_ssh_help(menu) or the existing help output routine) then
return; place this check alongside the _handle_ssh_list and _handle_ssh_kill
branches so detailed SSH help (including --window and --block option docs) is
available without a selected agent.
In `@praetorian_cli/ui/aegis/menu.py`:
- Line 444: Currently the code calls stop_all_proxies() before
self._tmux.kill_all(), which severs tunnel transports while panes are still
running; swap the order so self._tmux.kill_all() is invoked before
stop_all_proxies() to ensure tmux panes are terminated prior to shutting down
proxies (locate the callsite that calls stop_all_proxies() and replace the
sequence to call _tmux.kill_all() first, then stop_all_proxies()).
In `@praetorian_cli/ui/aegis/tmux.py`:
- Around line 207-242: In _create_native_pane add the detached flag to tmux
invocations and restore TUI focus after creating the pane: include "-d" in the
_tmux_cmd calls that create windows/panes (the "new-window" call when as_window
is True and both "split-window" calls when as_window is False) so creation is
non-blocking, and call self._refocus_tui() after the layout/option adjustments
(use a finally or after the try/except rebalance block) to ensure the TUI
regains focus; keep existing exception swallowing around layout calls to avoid
raising on tmux layout failures.
- Around line 256-267: The first-pane startup currently sends "exec {command}"
via self._tmux_cmd in the block guarded by if not self._panes, which replaces
the shell and prevents any trailing shell operators (like the wrapped "|| {
printf ...; read _; }") from running; remove the "exec" usage and instead
respawn the initial pane with the full compound command so the shell evaluates
the entire expression (use self._tmux_cmd("respawn-pane", "-t", pane_id,
command) or the equivalent respawn-pane invocation rather than send-keys with
exec), keeping the short time.sleep(0.2) and only using pane_id from the earlier
list-panes call.
---
Outside diff comments:
In `@praetorian_cli/ui/aegis/commands/ssh.py`:
- Around line 75-113: The parser is treating lowercase "-l" as a local-forward
flag so parse_ssh_args/SSHArgumentParser is turning "-l USER" into a
local_forward entry which later gets emitted as "-L USER"; update the parsing
rules in the SSHArgumentParser/parse_ssh_args implementation (and the underlying
ssh_utils parser) to reserve "-l" for the SSH login user (populate
parsed_options['user'] or similar) and only map "-L"/"--local" to
parsed_options['local_forward']; after that ensure the code that reconstructs
options uses parsed_options['user'] to emit "-l USER" (not as a forward) and
continues to emit "-L" for entries in parsed_options['local_forward'] so ssh -l
root becomes "-l root" instead of "-L root".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9baaaff3-feda-40cf-a074-f5950ff31882
📒 Files selected for processing (6)
praetorian_cli/handlers/ssh_utils.pypraetorian_cli/sdk/entities/aegis.pypraetorian_cli/ui/aegis/commands/help.pypraetorian_cli/ui/aegis/commands/ssh.pypraetorian_cli/ui/aegis/menu.pypraetorian_cli/ui/aegis/tmux.py
| # Route subcommands before requiring a selected agent | ||
| if len(args) and args[0].lower() == 'list': | ||
| _handle_ssh_list(menu) | ||
| return | ||
|
|
||
| if len(args) and args[0].lower() == 'kill': | ||
| _handle_ssh_kill(menu, args[1:]) | ||
| return |
There was a problem hiding this comment.
Route ssh help and ssh --help before the selected-agent guard.
Line 58 still rejects help when no agent is selected, so the detailed SSH help for list, kill, --window, and --block is inaccessible from a cold start.
Suggested fix
- if not menu.selected_agent:
- menu.console.print("\n No agent selected. Use 'set <id>' to select one.\n")
- menu.pause()
- return
-
# Accept `ssh help` too
if len(args) and args[0].lower() == 'help':
_print_help(menu)
menu.pause()
return
@@
if any(a in ('-h', '--help') for a in args):
_print_help(menu)
menu.pause()
return
+
+ if not menu.selected_agent:
+ menu.console.print("\n No agent selected. Use 'set <id>' to select one.\n")
+ menu.pause()
+ return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@praetorian_cli/ui/aegis/commands/ssh.py` around lines 49 - 56, The SSH
command currently routes 'list' and 'kill' before the selected-agent guard but
still rejects 'ssh help' / 'ssh --help'; add an early branch like the existing
ones that checks if len(args) and args[0].lower() in ('help', '--help', '-h')
and call the SSH help printer (create or reuse a helper such as
_show_ssh_help(menu) or the existing help output routine) then return; place
this check alongside the _handle_ssh_list and _handle_ssh_kill branches so
detailed SSH help (including --window and --block option docs) is available
without a selected agent.
| continue | ||
| finally: | ||
| stop_all_proxies(self) | ||
| self._tmux.kill_all() |
There was a problem hiding this comment.
Kill tmux panes before shutting down proxies.
Because stop_all_proxies() runs immediately before this, any pane still riding a SOCKS tunnel from praetorian_cli/ui/aegis/commands/proxy.py:278-301 loses its transport before the pane is terminated. Flipping the order makes shutdown cleaner.
Suggested fix
finally:
- stop_all_proxies(self)
self._tmux.kill_all()
+ stop_all_proxies(self)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@praetorian_cli/ui/aegis/menu.py` at line 444, Currently the code calls
stop_all_proxies() before self._tmux.kill_all(), which severs tunnel transports
while panes are still running; swap the order so self._tmux.kill_all() is
invoked before stop_all_proxies() to ensure tmux panes are terminated prior to
shutting down proxies (locate the callsite that calls stop_all_proxies() and
replace the sequence to call _tmux.kill_all() first, then stop_all_proxies()).
| def _create_native_pane(self, as_window: bool, command: str) -> Optional[str]: | ||
| """Split inside the current tmux window. | ||
|
|
||
| *command* is run as the pane process; when it exits the pane closes. | ||
| """ | ||
| if as_window: | ||
| out = self._tmux_cmd( | ||
| "new-window", "-P", "-F", "#{pane_id}", command, | ||
| ) | ||
| return out | ||
|
|
||
| existing = len(self._panes) | ||
| if existing == 0: | ||
| # First pane: horizontal split, SSH on the right at ~70% | ||
| out = self._tmux_cmd( | ||
| "split-window", "-h", "-l", "70%", "-P", "-F", "#{pane_id}", | ||
| command, | ||
| ) | ||
| else: | ||
| # Alternate vertical / horizontal splits for subsequent panes | ||
| flag = "-v" if existing % 2 == 1 else "-h" | ||
| out = self._tmux_cmd( | ||
| "split-window", flag, "-P", "-F", "#{pane_id}", | ||
| command, | ||
| ) | ||
|
|
||
| # Rebalance: main-vertical keeps TUI on the left at ~30% | ||
| try: | ||
| self._tmux_cmd("select-layout", "main-vertical") | ||
| self._tmux_cmd( | ||
| "set-window-option", "main-pane-width", "30%", | ||
| ) | ||
| except Exception: | ||
| pass | ||
|
|
||
| return out |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the tmux.py file and examine its full content
fd tmux.py --type fRepository: praetorian-inc/praetorian-cli
Length of output: 104
🏁 Script executed:
# Search for _refocus_tui method
rg "_refocus_tui" -A 5 -B 5Repository: praetorian-inc/praetorian-cli
Length of output: 846
🏁 Script executed:
# Examine the tmux.py file to check line numbers and context
cat -n praetorian_cli/ui/aegis/tmux.py | head -300Repository: praetorian-inc/praetorian-cli
Length of output: 11966
Add -d flag and refocus TUI in native tmux mode.
Lines 214, 222, and 229 create panes/windows without -d, which steals focus from the TUI. The _refocus_tui() method exists but is never called in _create_native_pane(). Without both fixes, native mode breaks the non-blocking TUI behavior.
Suggested fix
def _create_native_pane(self, as_window: bool, command: str) -> Optional[str]:
"""Split inside the current tmux window.
@@
if as_window:
out = self._tmux_cmd(
- "new-window", "-P", "-F", "#{pane_id}", command,
+ "new-window", "-d", "-P", "-F", "#{pane_id}", command,
)
- return out
+ self._refocus_tui()
+ return out
@@
out = self._tmux_cmd(
- "split-window", "-h", "-l", "70%", "-P", "-F", "#{pane_id}",
+ "split-window", "-d", "-h", "-l", "70%", "-P", "-F", "#{pane_id}",
command,
)
else:
# Alternate vertical / horizontal splits for subsequent panes
flag = "-v" if existing % 2 == 1 else "-h"
out = self._tmux_cmd(
- "split-window", flag, "-P", "-F", "#{pane_id}",
+ "split-window", "-d", flag, "-P", "-F", "#{pane_id}",
command,
)
@@
except Exception:
pass
+
+ self._refocus_tui()
return out🧰 Tools
🪛 Ruff (0.15.7)
[error] 239-240: try-except-pass detected, consider logging the exception
(S110)
[warning] 239-239: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@praetorian_cli/ui/aegis/tmux.py` around lines 207 - 242, In
_create_native_pane add the detached flag to tmux invocations and restore TUI
focus after creating the pane: include "-d" in the _tmux_cmd calls that create
windows/panes (the "new-window" call when as_window is True and both
"split-window" calls when as_window is False) so creation is non-blocking, and
call self._refocus_tui() after the layout/option adjustments (use a finally or
after the try/except rebalance block) to ensure the TUI regains focus; keep
existing exception swallowing around layout calls to avoid raising on tmux
layout failures.
| if not self._panes: | ||
| # Reuse the initial pane that new-session created. | ||
| # Use ``exec`` to replace the shell so the pane closes on exit. | ||
| out = self._tmux_cmd( | ||
| "list-panes", "-t", "aegis", "-F", "#{pane_id}", | ||
| ) | ||
| pane_id = out.splitlines()[0] if out else None | ||
| if pane_id: | ||
| time.sleep(0.2) | ||
| self._tmux_cmd( | ||
| "send-keys", "-t", pane_id, f"exec {command}", "Enter", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and inspect the tmux.py file
find . -name "tmux.py" -type fRepository: praetorian-inc/praetorian-cli
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Find and inspect the ssh.py file
find . -name "ssh.py" -type fRepository: praetorian-inc/praetorian-cli
Length of output: 114
🏁 Script executed:
cat -n ./praetorian_cli/ui/aegis/tmux.py | sed -n '250,275p'Repository: praetorian-inc/praetorian-cli
Length of output: 1222
🏁 Script executed:
cat -n ./praetorian_cli/ui/aegis/commands/ssh.py | sed -n '250,265p'Repository: praetorian-inc/praetorian-cli
Length of output: 698
🏁 Script executed:
# Check broader context of create_pane method and tmux usage
cat -n ./praetorian_cli/ui/aegis/tmux.py | sed -n '200,280p'Repository: praetorian-inc/praetorian-cli
Length of output: 3490
🏁 Script executed:
# Check how create_pane is called from ssh.py
cat -n ./praetorian_cli/ui/aegis/commands/ssh.py | sed -n '240,275p'Repository: praetorian-inc/praetorian-cli
Length of output: 1547
🏁 Script executed:
# Check if respawn-pane is used elsewhere
rg "respawn-pane" .Repository: praetorian-inc/praetorian-cli
Length of output: 55
🏁 Script executed:
# Check the full create_pane method signature to understand parameters
cat -n ./praetorian_cli/ui/aegis/tmux.py | sed -n '130,160p'Repository: praetorian-inc/praetorian-cli
Length of output: 1368
🏁 Script executed:
# Find the create_pane method
rg -A 15 "def create_pane" ./praetorian_cli/ui/aegis/tmux.pyRepository: praetorian-inc/praetorian-cli
Length of output: 730
🏁 Script executed:
# Also check how split-window passes commands for other panes
# Look at lines 273-276 again to understand the command parameter flow
cat -n ./praetorian_cli/ui/aegis/tmux.py | sed -n '100,120p'Repository: praetorian-inc/praetorian-cli
Length of output: 849
Remove exec prefix from first external-pane command.
The SSH command is wrapped with || { printf ...; read _; } to keep the pane open on failure, but prepending exec replaces the shell before the error handler runs. Once exec'd, the SSH process is the shell—there's nothing left to evaluate the || operator on failure.
The suggested fix using respawn-pane aligns with how subsequent panes execute commands (via split-window), allowing the compound command to work as intended.
Suggested fix
if not self._panes:
# Reuse the initial pane that new-session created.
- # Use ``exec`` to replace the shell so the pane closes on exit.
out = self._tmux_cmd(
"list-panes", "-t", "aegis", "-F", "#{pane_id}",
)
pane_id = out.splitlines()[0] if out else None
if pane_id:
- time.sleep(0.2)
- self._tmux_cmd(
- "send-keys", "-t", pane_id, f"exec {command}", "Enter",
- )
+ self._tmux_cmd(
+ "respawn-pane", "-k", "-t", pane_id, command,
+ )
return pane_id🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@praetorian_cli/ui/aegis/tmux.py` around lines 256 - 267, The first-pane
startup currently sends "exec {command}" via self._tmux_cmd in the block guarded
by if not self._panes, which replaces the shell and prevents any trailing shell
operators (like the wrapped "|| { printf ...; read _; }") from running; remove
the "exec" usage and instead respawn the initial pane with the full compound
command so the shell evaluates the entire expression (use
self._tmux_cmd("respawn-pane", "-t", pane_id, command) or the equivalent
respawn-pane invocation rather than send-keys with exec), keeping the short
time.sleep(0.2) and only using pane_id from the earlier list-panes call.
1 participant
Summary
TmuxBackendclass: Manages pane lifecycle with native mode (inside tmux) and external mode (standalone socket)ssh listshows active panes with uptime,ssh kill <id|all>terminates them--window/--blockflags: Open SSH in a new tmux window or force blocking mode (old behavior)build_ssh_commandSDK method: Returns shell-safe SSH command string viashlex.joinfor tmux integrationssh_utils.validate_agent_for_sshnow handles"null"string from backendTest plan
sshin TUI inside tmux — verify split pane opens, TUI stays usablessh --window— verify new tmux window opensssh --block— verify old blocking behaviorssh list— verify active pane table with uptimessh kill 1/ssh kill all— verify pane cleanupsshoutside tmux — verify external session on private socketsshwithout tmux installed — verify fallback to blocking modessh list,ssh kill,--window,--block