feat(cli): add --output json/yaml to sandbox get, status, and sandbox create#1
Closed
rhuss wants to merge 20 commits into
Closed
feat(cli): add --output json/yaml to sandbox get, status, and sandbox create#1rhuss wants to merge 20 commits into
rhuss wants to merge 20 commits into
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review 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 refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (104)
📝 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
June 24, 2026 13:18
16cc09b to
56049ac
Compare
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
3 times, most recently
from
June 24, 2026 16:32
02ec0f6 to
1816278
Compare
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
rhuss
force-pushed
the
feat/cli-structured-output
branch
3 times, most recently
from
June 25, 2026 05:54
b88a214 to
f067c05
Compare
Remove the production-crate dev-settings feature so e2e tests build the same gateway, CLI, and supervisor binaries that we ship. Move settings coverage from dummy test-only keys to the production ocsf_json_enabled setting. Signed-off-by: Simon Scatton <sscatton@nvidia.com>
* fix(docker): honor configured supervisor image Signed-off-by: Evan Lezar <elezar@nvidia.com> * fix(cli): isolate ssh from host linker environment Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Evan Lezar <elezar@nvidia.com>
Bumps [azure/setup-helm](https://github.com/azure/setup-helm) from 5.0.0 to 5.0.1. - [Release notes](https://github.com/azure/setup-helm/releases) - [Changelog](https://github.com/Azure/setup-helm/blob/main/CHANGELOG.md) - [Commits](Azure/setup-helm@dda3372...9bc31f4) --- updated-dependencies: - dependency-name: azure/setup-helm dependency-version: 5.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ateConfig struct (NVIDIA#1997) Signed-off-by: Yuedong Wu <dwcn22@outlook.com>
…r-socket (NVIDIA#1703) * feat(server): support remote compute driver endpoints Add named remote compute driver endpoint support to the gateway. Remote drivers are selected by a non-reserved compute driver name and either a CLI/env socket endpoint or [openshell.drivers.<name>].socket_path. The VM driver now enters ComputeRuntime through the same acquired remote endpoint path, while Docker, Podman, and Kubernetes retain their in-process drivers. Require --drivers/OPENSHELL_DRIVERS when pairing an ad-hoc socket endpoint so the socket does not imply a magic driver name, and keep reserved in-tree names unavailable for unmanaged socket endpoints. Co-authored-by: Evan Lezar <elezar@nvidia.com> Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com> Signed-off-by: Evan Lezar <elezar@nvidia.com> * test(server): cover remote compute driver UDS lifecycle Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com> Signed-off-by: Evan Lezar <elezar@nvidia.com> Co-authored-by: Evan Lezar <elezar@nvidia.com>
* fix(server): update driver spec test argument Signed-off-by: Evan Lezar <elezar@nvidia.com> * test(server): stabilize concurrent lease steal test Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Closes NVIDIA#1982 Reject user-authored _provider_* network policy keys at gateway boundaries, strip provider-derived rules from sandbox sync, and document the round-trip workflow. Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
…ution readiness (NVIDIA#1968) * fix(openshell-network-supervisor): gate proxy accept on symlink resolution readiness Signed-off-by: Calum Murray <cmurray@redhat.com> * fix(openshell-network-supervisor): skip symlink resolution gate when process supervisor is disabled Signed-off-by: Calum Murray <cmurray@redhat.com> --------- Signed-off-by: Calum Murray <cmurray@redhat.com>
…nnels (NVIDIA#1638) Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> Co-authored-by: Russell Bryant <russell.bryant@gmail.com>
* feat(kubernetes): support agent-sandbox v1beta1 Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * ci(kubernetes): test agent-sandbox api versions Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): retry agent-sandbox raw 404s Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): harden agent-sandbox api setup Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): cache agent-sandbox api version Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * docs(kubernetes): document agent-sandbox upgrade behavior Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * refactor(kubernetes): collapse agent-sandbox api selection Signed-off-by: Taylor Mutch <taylormutch@gmail.com> --------- Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
…IDIA#1992) Add HTTP/2 keepalive on supervisor multiplex connections so half-dead sessions cannot leave in-flight exec relays parked indefinitely. Configure SSH keepalive on exec relay clients so long silent commands are not timed out on stdout idle alone; wedged or orphaned relays fail after missed keepalives instead. After a command reports exit status, bound how long the gateway waits for the trailing channel close. Return UNAVAILABLE when a relay closes before reporting exit status rather than defaulting to exit code 1. Signed-off-by: Gal Zaidman <gzaidman@nvidia.com>
Signed-off-by: Taylor Mutch <taylormutch@gmail.com>
The generated protobuf/gRPC modules under python/openshell/_proto/ are gitignored. maturin honors .gitignore when collecting python-source files from a git checkout, so native builds (Linux release CI, local pip install .) dropped these modules and shipped an unimportable wheel. The macOS wheel only included them by accident: it builds in a Docker context whose COPY omits .git, so maturin had no gitignore to apply. Pin the stubs back in with explicit, package-relative [tool.maturin].include globs so inclusion no longer depends on whether .git is present in the build context. Add a wheel smoke check to the release-tag and release-dev workflows that installs each Linux wheel in a clean python:3.12-slim image and imports openshell.sandbox, and document the invariant in architecture/build.md. Closes NVIDIA#1705 Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>
Add policy schema, proto, provider profile, OPA, and L7 proxy support for `protocol: json-rpc` and `protocol: mcp`. Generic JSON-RPC endpoints match exact method names only, with `method: "*"` as the all-method sentinel; wildcard/glob methods and params matchers are rejected. Parse JSON-RPC request bodies and batches in the forward proxy, deny response-shaped client frames, limit receive-stream GET allowance to MCP endpoints, and redact params in decision logs. Preserve L7 rule params on the proto load path so MCP `tools/call` tool filters behave like YAML-loaded policies. Add MCP conformance coverage, JSON-RPC L7 e2e coverage, and docs for the new protocols and current matcher limitations. Signed-off-by: Kris Hicks <khicks@nvidia.com> Co-authored-by: ddurst <267424412+ddurst-nvidia@users.noreply.github.com>
… 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
June 29, 2026 07:39
f067c05 to
d287542
Compare
rhuss
added a commit
that referenced
this pull request
Jul 15, 2026
- Remove dead boolCount function that would fail golangci-lint (#1) - Emit EventAdded for the first watch event instead of EventModified, matching k8s watch semantics (#7) - Add mutex locking to all mock server methods that access the shared sandboxes map, fixing latent race conditions (#12) - Skip HealthCheck integration test that calls an unimplemented stub (#13) - Scope doc.go examples: mark sections for sub-clients not yet available in this PR with "available in a future release" (#4) - Document Config.Timeout/RetryPolicy/Logger and WatchOptions fields as reserved for future use (#2, #6) Signed-off-by: Roland Huß <rhuss@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.