feat(cli): add --output json/yaml to sandbox get, status, and sandbox create#1
feat(cli): add --output json/yaml to sandbox get, status, and sandbox create#1rhuss wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 43 minutes and 49 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds ChangesStructured Output Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rhuss
force-pushed
the
feat/cli-structured-output
branch
from
16cc09b to
56049ac
Compare
![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
🧹 Nitpick comments (1)
crates/openshell-cli/src/main.rs (1)
536-540: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd parse tests for the new
--outputcontracts.This file adds new CLI surface for
status,sandbox create, andsandbox get, but the visible test change at Line 3483 only relaxes theStatusvariant shape. Please add focused clap tests for defaulttable,-o json/-o yaml, andsandbox get --policy-onlyconflicting with--output, mirroring the existing list/provider coverage below.Also applies to: 1343-1365, 3483-3483
🤖 Prompt for 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. In `@crates/openshell-cli/src/main.rs` around lines 536 - 540, Add focused clap parse tests for the new output flags on the existing command structs, since the current test update only loosens the Status variant shape. In the test module covering Status, SandboxCreate, and SandboxGet, verify the default OutputFormat::Table for status, the -o json and -o yaml parses, and that SandboxGet with --policy-only rejects --output as a conflict. Reuse the same style and helpers already used for the list/provider CLI coverage so the new contracts are exercised consistently.
🤖 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/openshell-cli/src/main.rs`:
- Around line 2095-2103: The gateway status fallback is too broad because
resolve_gateway can fail for reasons other than no active gateway, so
non-configuration errors are being collapsed into not_configured. Update the
gateway_status flow in main to only use the JSON fallback when resolve_gateway
indicates the no-active-gateway case, and let unknown gateway or missing
metadata errors propagate normally through the existing gateway status path.
Keep the fix localized around resolve_gateway, apply_auth, and
run::gateway_status so structured callers still receive real resolution
failures.
In `@crates/openshell-cli/src/run.rs`:
- Around line 3503-3512: The policy serialization flow in run.rs currently
swallows failures by turning any `serialize_sandbox_policy(...)` or YAML-to-JSON
parse error into `serde_json::Value::Null`, which makes an existing policy look
like “no policy configured.” Update the logic around `config.policy`,
`serialize_sandbox_policy`, and the `obj.insert("policy", ...)` path so the JSON
value is constructed before `print_output_single(...)` and any conversion
failure is returned as an error instead of defaulting to null, matching the
behavior of the `--policy-only` path.
- Around line 2302-2304: The structured_output early return in run should not
bypass the --editor and trailing command paths. Update the logic around
print_output_single so that openshell sandbox create with --output json plus
--editor or a trailing command either errors out before this branch or continues
into the existing session/command execution flow. Use the structured_output flag
and the surrounding editor/trailing-command handling in run to locate and fix
the control flow.
In `@docs/sandboxes/manage-sandboxes.mdx`:
- Around line 23-27: The sandbox creation docs currently advertise only JSON
output, but `sandbox create` also supports YAML. Update the
`manage-sandboxes.mdx` usage guidance for `sandbox create` to match the wording
used for `sandbox status` and `sandbox get`, and mention both `--output json`
and `--output yaml` so the CLI surface is discoverable and consistent.
---
Nitpick comments:
In `@crates/openshell-cli/src/main.rs`:
- Around line 536-540: Add focused clap parse tests for the new output flags on
the existing command structs, since the current test update only loosens the
Status variant shape. In the test module covering Status, SandboxCreate, and
SandboxGet, verify the default OutputFormat::Table for status, the -o json and
-o yaml parses, and that SandboxGet with --policy-only rejects --output as a
conflict. Reuse the same style and helpers already used for the list/provider
CLI coverage so the new contracts are exercised consistently.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c2e30fc-362a-4990-9c76-e7d21c170bf9
📒 Files selected for processing (6)
crates/openshell-cli/src/main.rscrates/openshell-cli/src/run.rscrates/openshell-cli/tests/sandbox_create_lifecycle_integration.rscrates/openshell-cli/tests/sandbox_name_fallback_integration.rsdocs/sandboxes/manage-gateways.mdxdocs/sandboxes/manage-sandboxes.mdx
There was a problem hiding this comment.
Pull request overview
Adds structured --output json|yaml support to key OpenShell CLI flows to enable scripting/automation while preserving existing human-readable output by default.
Changes:
- Added
--outputflag handling foropenshell status,openshell sandbox get, andopenshell sandbox create. - Implemented JSON/YAML converters for gateway status and sandbox detail (including policy metadata/content).
- Updated docs and integration tests to reflect new CLI flag surface and signatures.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/sandboxes/manage-sandboxes.mdx | Documents `--output json |
| docs/sandboxes/manage-gateways.mdx | Documents `--output json |
| crates/openshell-cli/tests/sandbox_name_fallback_integration.rs | Updates test helpers/calls to pass the new output parameter for sandbox get. |
| crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs | Updates sandbox create integration tests for the new output parameter. |
| crates/openshell-cli/src/run.rs | Implements structured output for status/sandbox commands and adds JSON conversion helpers. |
| crates/openshell-cli/src/main.rs | Wires new --output flags into clap command definitions and dispatch to run::* functions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rhuss
force-pushed
the
feat/cli-structured-output
branch
5 times, most recently
from
9aa0ec4 to
b88a214
Compare
… create Add structured output support to three commands that previously only supported human-readable table output: - `sandbox get --output json/yaml`: emits sandbox detail including policy source, revision, and active policy content via a new `sandbox_detail_to_json` converter. - `status --output json/yaml`: emits gateway connection state via a new `status_to_json` converter with conditional fields (auth, version, error, http_status) matching the human output. - `sandbox create --output json/yaml`: emits sandbox metadata after the sandbox reaches Ready phase, suppressing spinners and ANSI chrome from stdout. Uploads and port forwarding still execute before output. Clap rejects `--output` combined with `--editor` or trailing commands. All three commands reuse the existing `OutputFormat` enum and `print_output_single` helper. Default output (no --output flag) is byte-identical to current behavior. Unit tests cover both converters. Closes NVIDIA#1964 Assisted-By: 🤖 Claude Code Signed-off-by: Roland Huß <rhuss@redhat.com>
rhuss
force-pushed
the
feat/cli-structured-output
branch
from
b88a214 to
f067c05
Compare
Summary
Add
--output json/yamlsupport to three CLI commands that previously only had human-readable output:sandbox get: Structured sandbox detail with policy source, revision, and active policy contentstatus: Gateway connection state with conditional fields (auth, version, error, http_status)sandbox create: Sandbox metadata emitted after Ready phase, human chrome suppressed from stdoutAll three commands reuse the existing
OutputFormatenum andprint_output_singlehelper. Default behavior (no--outputflag) is byte-identical to current output.Test plan
cargo test --lib)sandbox_detail_to_jsonandstatus_to_jsonconverterscargo clippyclean,cargo fmtcleansandbox get --output json,status --output json/yaml, default output regression check (byte-identical)Closes NVIDIA#1964
Assisted-By: 🤖 Claude Code
Summary by CodeRabbit
--output/-o(default:table) forgateway status,sandbox create, andsandbox getto enable machine-readable JSON/YAML.gateway statusnow returns structured output (e.g.{"status":"not_configured"}) when no gateway is configured.sandbox get:--policy-onlyis mutually exclusive with--output;sandbox createuses structured output during provisioning (skipping the interactive session flow).--output json/--output yamlautomation examples.