feat(min): move session exec to own subcommand - #1186
Conversation
📝 WalkthroughWalkthroughThe CLI adds ChangesSession execution flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant cmd_exec
participant session_via_ssh
participant SSH
CLI->>cmd_exec: receive session and command arguments
cmd_exec->>session_via_ssh: resolve session and invoke SSH
session_via_ssh->>SSH: forward separate command arguments
SSH-->>CLI: return command output and exit status
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/minimal-client/src/attach.rs`:
- Around line 133-134: Quote each command element in the loop building the SSH
remote command in crates/minimal-client/src/attach.rs, preserving argument
boundaries and shell safety. Update minimald::handle_exec to unquote or
otherwise normalize the routed command before applying the existing “min ”
prefix routing. In scripts/session-e2e.sh lines 318-332, invoke the shell cases
as argument vectors using sh -c with “echo EXEC_OK $PWD” and “exit 7”.
In `@crates/minimal/src/lib.rs`:
- Around line 207-209: Update the command argument definition in the session
exec CLI options to set required = true, ensuring an empty command is rejected
before reaching session_via_ssh. Keep trailing_var_arg behavior and existing
command parsing intact.
In `@docs/concepts/sessions.md`:
- Around line 235-237: Update the session execution documentation around the
`min session exec` example to state that `ExecArgs` supports arbitrary commands
and arguments non-interactively. Replace the `<session> min run <task>`-specific
syntax with `min session exec <session> <command> [args...]`, and remove the
claim that arbitrary commands require an interactive shell.
🪄 Autofix
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: fb571f08-3f71-47d6-8bb5-48d3d3fb9418
📒 Files selected for processing (8)
crates/minimal-client/src/attach.rscrates/minimal-tui/src/app.rscrates/minimal/src/lib.rscrates/minimal/src/main.rscrates/minimal/tests/cli.rscrates/minimald/src/guest.rsdocs/concepts/sessions.mdscripts/session-e2e.sh
5a0a6a6 to
d65a2d9
Compare
min session attach <sid> -cmin session exec <sid> <args>Summary by CodeRabbit
New Features
min session exec <session> <command...>for running commands in sessions.Changes
session attach; usesession execfor non-interactive commands.Documentation
Note
Add
min session execsubcommand for non-interactive command execution in sessionsSessionCommand::Execvariant andExecArgsstruct, addingmin session exec <session> <command...>as a dedicated CLI subcommand for running commands non-interactively in an existing session via SSH.commandoption frommin session attach; attach is now purely interactive.attach_to_sessiontosession_via_sshin lib.rs, accepting aVec<String>command; when empty, it performs an interactive attach (with TTY check), otherwise a non-interactive exec.stdout_is_data_contractin main.rs to treatsession exec(notsession attach) as the stdout contract path.min session attachno longer accepts a command argument; callers must switch tomin session exec.Macroscope summarized d65a2d9.