Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional CLI for the aviato SDK, providing terminal-based commands for common sandbox operations. The CLI is implemented as a Click-based subpackage available via the [cli] extra dependency, ensuring that importing the main aviato package doesn't require Click.
Changes:
- Adds
aviato listandaviato execcommands for listing and executing commands in sandboxes - Implements proper ImportError handling with helpful install instructions when Click is missing
- Includes comprehensive test coverage for CLI commands, entry points, and error handling
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Adds CLI console script entry point, cli optional dependency group, and includes click in dev dependencies |
| uv.lock | Updates lock file with Click dependency and new version 0.1.1 |
| src/aviato/main.py | Entry point for python -m aviato and aviato console script with ImportError fallback handling |
| src/aviato/init.py | Updates version to 0.1.1 |
| src/aviato/cli/init.py | Click group definition with version option and command registration |
| src/aviato/cli/list.py | List command with filtering by status, tags, runway IDs, and tower IDs |
| src/aviato/cli/exec.py | Exec command with real-time stdout/stderr streaming, cwd and timeout options |
| tests/unit/aviato/test_cli_main.py | Tests for CLI entry point, help, version, and ImportError handling |
| tests/unit/aviato/test_cli_list.py | Tests for list command registration, output format, filters, and error handling |
| tests/unit/aviato/test_cli_exec.py | Tests for exec command with stdout/stderr streaming, exit codes, options, and error scenarios |
| tests/unit/aviato/conftest.py | Adds event loop management for testing concurrent stdout/stderr streaming |
| docs/guides/execution.md | Documents CLI exec command with examples |
| DEVELOPMENT.md | Adds CLI usage documentation for developers |
| AGENTS.md | Documents CLI architecture and conventions |
| src/aviato/AGENTS.md | Updates file structure to include CLI package |
| tests/AGENTS.md | Adds CLI test files to test inventory |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3b91b0e to
3e75067
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3e75067 to
bfa8efe
Compare
3ec8591 to
151f10b
Compare
794d0c2 to
4818f32
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4818f32 to
54d0998
Compare
NavarrePratt
left a comment
There was a problem hiding this comment.
[via Claude]
Branch Review: exian/cli-core
Overview
- Branch: exian/cli-core -> exian/fixes
- Commits: 3 commits
- Files Changed: 18 files (+667/-39 lines)
- Review Team: 4 Claude reviewers (Opus) + Codex validation
- Reviewers: Security, Correctness, Architecture, Simplicity
Outcome: APPROVED
No confirmed Critical or High findings after Codex validation. Two Medium findings were identified; one was already fixed in the latest commits (ImportError name attribute). One Medium finding remains (CLI error handling).
Full review report
Critical & High Findings
None.
Medium Findings
1. ImportError name check (FIXED)
- File: src/aviato/cli/init.py:19
- Status: Fixed in latest commits -
name="click"now set on re-raised ImportError - Found by: reviewer-architecture, reviewer-simplicity, reviewer-correctness
2. No CLI-level error handling for SDK exceptions
- File: src/aviato/cli/exec.py:50, src/aviato/cli/list.py:37
- Status: Still present - inline comment posted
- Found by: reviewer-security, reviewer-architecture
Low Findings
3. stderr drain truncation (Skipped)
- File: src/aviato/cli/exec.py:63,70
- Status: Standard thread pattern with pragmatic timeout; skipped after industry comparison
- Found by: reviewer-correctness
4. Test validates wrong failure path (FIXED)
- File: tests/unit/aviato/test_cli_main.py:47-63
- Status: Fixed - new test_main_import_error_missing_click test added
- Found by: reviewer-simplicity (Codex)
Review Statistics
- Total findings (pre-validation): 12
- Post-validation: 4 confirmed, 5 disputed, 2 severity-adjusted, 1 new from Codex
- By severity: Critical: 0, High: 0, Medium: 2, Low: 2
- Cross-reviewer overlap: 3 findings caught by 2+ reviewers
- Claude-Codex agreement rate: 75%
Reviewer Notes
Security: Overall security posture solid. Shell injection prevented with shlex.quote(). Auth flows through gRPC infrastructure. No secrets in code.
Correctness: No data corruption, crash, or silent wrong-result risks. Concurrency model sound.
Architecture: Clean module boundary. CLI subpackage design is architecturally sound.
Simplicity: Excellent simplicity. No over-engineering detected.
54d0998 to
5a5835e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5a5835e to
cb9e2ee
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cb9e2ee to
921b0ec
Compare
151f10b to
39a869e
Compare
921b0ec to
4a8a0ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4a8a0ab to
3ec8f84
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3ec8f84 to
0ce07ce
Compare
39a869e to
0b4e04a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0ce07ce to
4f8bbf2
Compare
NavarrePratt
left a comment
There was a problem hiding this comment.
[via Claude] The existing examples/ and docs/guides/ are strong. I see the later stack PRs (logs, shell) add logging.md, interactive-shells.md, and a new example script. A CLI-specific quickstart guide (installing the extra, common workflows like list -> exec -> logs) would round this out nicely as a follow-up after the full stack merges. Not blocking for this PR.
src/aviato/cli/__init__.py
Outdated
|
|
||
|
|
||
| cli.add_command(exec_command, "exec") | ||
| cli.add_command(list_sandboxes) |
There was a problem hiding this comment.
[via Claude] Consider adding command aliases for common shorthand. Click supports registering the same command under multiple names:
cli.add_command(list_sandboxes)
cli.add_command(list_sandboxes, "ls")ls for list is the most natural one. Could also consider run as an alias for exec if that feels right for the UX.
There was a problem hiding this comment.
Will add ls as an alias for list in this PR. Skipping run as an alias for exec for now.
In the SDK, Sandbox.run() creates a new sandbox (one-shot) while sandbox.exec() runs commands in an existing one (repeatable). Aliasing the CLI exec to run could create confusion between these two distinct operations, especially for users who use both the SDK and CLI.
| click.echo("No sandboxes found.") | ||
| return | ||
|
|
||
| click.echo(f"{'SANDBOX ID':<40} {'STATUS':<14} {'TOWER':<20} {'RUNWAY':<20} {'STARTED AT'}") |
There was a problem hiding this comment.
[via Claude] The tabular output covers the basics but users will likely want more detail for individual sandboxes (tags, resources, network config, full status history). A few ideas for follow-up after this stack lands:
aviato describe <sandbox-id>for detailed single-sandbox view--output jsonon list for machine-readable output--output widefor additional columns (tags, returncode, etc.)
There was a problem hiding this comment.
Are we able to surface the tags? I wonder if it requires an aviato change. WIll file a follow up!
There was a problem hiding this comment.
There's a number of things we usually store within the client sandbox instances that we only get when we create it. You can see what we get and don't get in the sandbox instance creation flow from the list/from-existing flow.
There was a problem hiding this comment.
follow up: #65 needed for --wide and describe
0b4e04a to
1daf25b
Compare
4f8bbf2 to
0dcc0de
Compare
23efa22 to
9398b37
Compare
Managing sandboxes currently requires writing Python scripts for every operation. A CLI enables quick terminal workflows — listing, executing, and inspecting sandboxes — without boilerplate. Add the cwsandbox CLI framework as an optional dependency with a Click group, __main__ entry point, and ImportError fallback when Click is not installed.
Add `cwsandbox ls` with table and JSON output formats. Supports filtering by --status, --tag, --runway-id, and --tower-id.
Add `cwsandbox exec <sandbox-id> <command>` for running commands in sandboxes with real-time stdout/stderr streaming. Supports --cwd and --timeout options. Update conftest make_process helper to use real event loops for thread-safe stream iteration in tests.
Cover installation, ls/exec usage, and JSON output for scripting.
9398b37 to
154832d
Compare
Summary
Introduces the
cwsandboxCLI as an optional Click-based subpackage (pip install cwsandbox[cli]), withlsandexeccommands for terminal-based sandbox management.cwsandbox.cli, console script entry point,__main__.pywith narrowedImportErrorhandling that re-raises unrelated import failurescwsandbox ls— Query sandboxes with--status,--tag,--runway-id,--tower-idfilters; table and JSON outputcwsandbox exec— Execute commands in a sandbox with real-time stdout/stderr streaming,--cwdand--timeoutoptions, exit code passthroughDesign decisions
cliextra —import cwsandboxnever loads Click or CLI modulescwsandbox.cli.<cmd>.Sandboxin tests (not the top-levelcwsandbox.Sandbox) for proper isolationclick>=8.0directly instead of self-referencingcwsandbox[cli]to avoid resolver confusionTest plan
test_cli_main.py— help, version, ImportError fallback, unrelated ImportError re-raisetest_cli_list.py— registration, output format, filters, empty state, API errorstest_cli_exec.py— stdout/stderr streaming, exit codes,--cwd,--timeout, not-found errors