feat(cli): add structured output to list commands (#3067)

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
This commit is contained in:
Mrunal Patel
2026-09-01 16:16:01 +00:00
committed by GitHub
parent a4f9c762ce
commit d5742e01a3
10 changed files with 765 additions and 36 deletions
+224 -13
View File
@@ -742,7 +742,7 @@ fn normalize_completion_script(output: Vec<u8>, executable: &std::path::Path) ->
Ok(script.replace(executable.to_string_lossy().as_ref(), "openshell"))
}
#[derive(Clone, Debug, ValueEnum)]
#[derive(Clone, Debug, PartialEq, Eq, ValueEnum)]
enum OutputFormat {
Table,
Yaml,
@@ -1691,6 +1691,10 @@ enum SandboxProviderCommands {
/// Sandbox name (defaults to last-used sandbox).
#[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))]
name: Option<String>,
/// Output format.
#[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table)]
output: OutputFormat,
},
/// Attach a provider to a sandbox.
@@ -1905,6 +1909,10 @@ enum PolicyCommands {
/// List global policy revisions.
#[arg(long)]
global: bool,
/// Output format.
#[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table)]
output: OutputFormat,
},
/// Delete the gateway-global policy lock, restoring sandbox-level policy control.
@@ -2011,9 +2019,13 @@ enum ForwardCommands {
name: Option<String>,
},
/// List active port forwards.
/// List tracked port forwards.
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
List,
List {
/// Output format.
#[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table)]
output: OutputFormat,
},
/// Forward a local TCP port to a loopback service inside a sandbox over gRPC.
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
@@ -2071,6 +2083,10 @@ enum ServiceCommands {
/// List services across all workspaces (overrides --workspace).
#[arg(long)]
all_workspaces: bool,
/// Output format.
#[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table)]
output: OutputFormat,
},
/// Show one exposed sandbox service endpoint.
@@ -2195,6 +2211,10 @@ enum WorkspaceMemberCommands {
/// Offset into the member list.
#[arg(long, default_value_t = 0)]
offset: u32,
/// Output format.
#[arg(short = 'o', long = "output", value_enum, default_value_t = OutputFormat::Table)]
output: OutputFormat,
},
}
@@ -2445,8 +2465,23 @@ async fn run_async() -> Result<()> {
);
}
}
ForwardCommands::List => {
ForwardCommands::List { output } => {
let forwards = run::list_forwards()?;
if openshell_cli::output::print_output_collection(
output.as_str(),
&forwards,
|forward| {
serde_json::json!({
"sandbox": forward.sandbox_name,
"bind_address": forward.bind_addr,
"port": forward.port,
"pid": forward.pid,
"alive": forward.validated_alive,
})
},
)? {
return Ok(());
}
if forwards.is_empty() {
eprintln!("No active forwards.");
} else {
@@ -2574,6 +2609,7 @@ async fn run_async() -> Result<()> {
limit,
offset,
all_workspaces,
output,
} => {
run::service_list(
&ctx.endpoint,
@@ -2582,6 +2618,7 @@ async fn run_async() -> Result<()> {
offset,
&cli.workspace,
all_workspaces,
output.as_str(),
&tls,
)
.await?;
@@ -2745,14 +2782,28 @@ async fn run_async() -> Result<()> {
name,
limit,
global,
output,
} => {
if global {
run::sandbox_policy_list_global(&ctx.endpoint, limit, &cli.workspace, &tls)
.await?;
run::sandbox_policy_list_global(
&ctx.endpoint,
limit,
output.as_str(),
&cli.workspace,
&tls,
)
.await?;
} else {
let name = resolve_sandbox_name(name, &ctx.name, &cli.workspace)?;
run::sandbox_policy_list(&ctx.endpoint, &name, limit, &cli.workspace, &tls)
.await?;
run::sandbox_policy_list(
&ctx.endpoint,
&name,
limit,
output.as_str(),
&cli.workspace,
&tls,
)
.await?;
}
}
PolicyCommands::Delete { global, yes } => {
@@ -3289,10 +3340,16 @@ async fn run_async() -> Result<()> {
run::print_ssh_config(&ctx.name, &name, &cli.workspace);
}
SandboxCommands::Provider(command) => match command {
SandboxProviderCommands::List { name } => {
SandboxProviderCommands::List { name, output } => {
let name = resolve_sandbox_name(name, &ctx.name, &cli.workspace)?;
run::sandbox_provider_list(endpoint, &name, &cli.workspace, &tls)
.await?;
run::sandbox_provider_list(
endpoint,
&name,
output.as_str(),
&cli.workspace,
&tls,
)
.await?;
}
SandboxProviderCommands::Attach { name, provider } => {
run::sandbox_provider_attach(
@@ -3373,9 +3430,17 @@ async fn run_async() -> Result<()> {
workspace,
limit,
offset,
output,
} => {
run::workspace_member_list(endpoint, &workspace, limit, offset, &tls)
.await?;
run::workspace_member_list(
endpoint,
&workspace,
limit,
offset,
output.as_str(),
&tls,
)
.await?;
}
},
}
@@ -4527,6 +4592,152 @@ mod tests {
));
}
#[test]
fn forward_list_default_output_is_table() {
let cli = Cli::try_parse_from(["openshell", "forward", "list"])
.expect("forward list should parse");
assert!(matches!(
cli.command,
Some(Commands::Forward {
command: Some(ForwardCommands::List {
output: OutputFormat::Table,
})
})
));
}
#[test]
fn forward_list_accepts_structured_output() {
for (format, expected) in [("json", OutputFormat::Json), ("yaml", OutputFormat::Yaml)] {
let cli = Cli::try_parse_from(["openshell", "forward", "list", "-o", format])
.unwrap_or_else(|error| panic!("forward list -o {format} should parse: {error}"));
let Some(Commands::Forward {
command: Some(ForwardCommands::List { output }),
}) = cli.command
else {
panic!("expected forward list command");
};
assert_eq!(output, expected, "unexpected output format for {format}");
}
}
#[test]
fn remaining_list_commands_default_output_to_table() {
let sandbox_provider = Cli::try_parse_from(["openshell", "sandbox", "provider", "list"])
.expect("sandbox provider list should parse");
assert!(matches!(
sandbox_provider.command,
Some(Commands::Sandbox {
command: Some(SandboxCommands::Provider(SandboxProviderCommands::List {
output: OutputFormat::Table,
..
})),
})
));
let policy =
Cli::try_parse_from(["openshell", "policy", "list"]).expect("policy list should parse");
assert!(matches!(
policy.command,
Some(Commands::Policy {
command: Some(PolicyCommands::List {
output: OutputFormat::Table,
..
}),
})
));
let service = Cli::try_parse_from(["openshell", "service", "list"])
.expect("service list should parse");
assert!(matches!(
service.command,
Some(Commands::Service {
command: Some(ServiceCommands::List {
output: OutputFormat::Table,
..
}),
})
));
let workspace_member = Cli::try_parse_from([
"openshell",
"workspace",
"member",
"list",
"--workspace",
"default",
])
.expect("workspace member list should parse");
assert!(matches!(
workspace_member.command,
Some(Commands::Workspace {
command: Some(WorkspaceCommands::Member(WorkspaceMemberCommands::List {
output: OutputFormat::Table,
..
})),
})
));
}
#[test]
fn remaining_list_commands_accept_structured_output() {
for (format, expected) in [("json", OutputFormat::Json), ("yaml", OutputFormat::Yaml)] {
let sandbox_provider =
Cli::try_parse_from(["openshell", "sandbox", "provider", "list", "-o", format])
.unwrap_or_else(|error| panic!("sandbox provider list -o {format}: {error}"));
let Some(Commands::Sandbox {
command:
Some(SandboxCommands::Provider(SandboxProviderCommands::List { output, .. })),
}) = sandbox_provider.command
else {
panic!("expected sandbox provider list command");
};
assert_eq!(output, expected);
let policy = Cli::try_parse_from(["openshell", "policy", "list", "--output", format])
.unwrap_or_else(|error| panic!("policy list --output {format}: {error}"));
let Some(Commands::Policy {
command: Some(PolicyCommands::List { output, .. }),
}) = policy.command
else {
panic!("expected policy list command");
};
assert_eq!(output, expected);
let service = Cli::try_parse_from(["openshell", "service", "list", "-o", format])
.unwrap_or_else(|error| panic!("service list -o {format}: {error}"));
let Some(Commands::Service {
command: Some(ServiceCommands::List { output, .. }),
}) = service.command
else {
panic!("expected service list command");
};
assert_eq!(output, expected);
let workspace_member = Cli::try_parse_from([
"openshell",
"workspace",
"member",
"list",
"--workspace",
"default",
"-o",
format,
])
.unwrap_or_else(|error| panic!("workspace member list -o {format}: {error}"));
let Some(Commands::Workspace {
command:
Some(WorkspaceCommands::Member(WorkspaceMemberCommands::List { output, .. })),
}) = workspace_member.command
else {
panic!("expected workspace member list command");
};
assert_eq!(output, expected);
}
}
#[test]
fn sandbox_list_default_output_is_table() {
let cli = Cli::try_parse_from(["openshell", "sandbox", "list"])
+287 -14
View File
@@ -2272,6 +2272,7 @@ fn sandbox_detail_to_json(
pub async fn sandbox_provider_list(
server: &str,
name: &str,
output: &str,
workspace: &str,
tls: &TlsOptions,
) -> Result<()> {
@@ -2285,6 +2286,10 @@ pub async fn sandbox_provider_list(
.into_diagnostic()?;
let providers = response.into_inner().providers;
if crate::output::print_output_collection(output, &providers, attached_provider_to_json)? {
return Ok(());
}
if providers.is_empty() {
println!("No providers attached to sandbox {name}.");
return Ok(());
@@ -2410,6 +2415,18 @@ fn print_provider_attachment_table(providers: &[Provider]) {
print!("{}", format_provider_attachment_table(providers, true));
}
fn attached_provider_to_json(provider: &Provider) -> serde_json::Value {
let mut config_keys = provider.config.keys().cloned().collect::<Vec<_>>();
config_keys.sort();
serde_json::json!({
"name": provider.object_name(),
"type": provider.r#type,
"credential_keys": provider_credential_keys(provider),
"config_keys": config_keys,
})
}
fn format_provider_attachment_table(providers: &[Provider], color: bool) -> String {
use std::fmt::Write as _;
@@ -3026,6 +3043,7 @@ fn service_expose_status_error(status: Status) -> miette::Report {
service_status_error("expose service", "sandbox:write", status)
}
#[allow(clippy::too_many_arguments)] // user-facing CLI command
pub async fn service_list(
server: &str,
sandbox: Option<&str>,
@@ -3033,6 +3051,7 @@ pub async fn service_list(
offset: u32,
workspace: &str,
all_workspaces: bool,
output: &str,
tls: &TlsOptions,
) -> Result<()> {
let mut client = grpc_client(server, tls).await?;
@@ -3052,6 +3071,15 @@ pub async fn service_list(
.map_err(|status| service_status_error("list services", "sandbox:read", status))?
.into_inner();
let services = response
.services
.iter()
.filter_map(|response| service_endpoint_to_json(response, server))
.collect::<Vec<_>>();
if crate::output::print_output_collection(output, &services, Clone::clone)? {
return Ok(());
}
if response.services.is_empty() {
if let Some(sandbox) = sandbox {
println!("No services exposed for sandbox {sandbox}.");
@@ -3240,6 +3268,30 @@ fn print_service_endpoint_table(
}
}
fn service_endpoint_to_json(
response: &ServiceEndpointResponse,
gateway_endpoint: &str,
) -> Option<serde_json::Value> {
let endpoint = response.endpoint.as_ref()?;
let workspace = endpoint
.metadata
.as_ref()
.map_or("", |metadata| metadata.workspace.as_str());
let url = if response.url.is_empty() {
String::new()
} else {
service_url_for_gateway(&response.url, gateway_endpoint)
};
Some(serde_json::json!({
"workspace": workspace,
"sandbox": endpoint.sandbox_name,
"service": endpoint.service_name,
"target_port": endpoint.target_port,
"url": url,
}))
}
fn service_display_name(service: &str) -> &str {
if service.is_empty() { "-" } else { service }
}
@@ -5340,9 +5392,10 @@ pub async fn workspace_member_list(
workspace: &str,
limit: u32,
offset: u32,
output: &str,
tls: &TlsOptions,
) -> Result<()> {
use openshell_core::proto::{ListWorkspaceMembersRequest, WorkspaceRole};
use openshell_core::proto::ListWorkspaceMembersRequest;
let mut client = grpc_client(server, tls).await?;
let response = client
@@ -5355,6 +5408,10 @@ pub async fn workspace_member_list(
.into_diagnostic()?;
let members = response.into_inner().members;
if crate::output::print_output_collection(output, &members, workspace_member_to_json)? {
return Ok(());
}
if members.is_empty() {
println!("No members found in workspace {workspace}.");
return Ok(());
@@ -5370,17 +5427,30 @@ pub async fn workspace_member_list(
println!("{:<subject_width$} {}", "SUBJECT".bold(), "ROLE".bold());
for member in &members {
let role_str = match WorkspaceRole::try_from(member.role) {
Ok(WorkspaceRole::Admin) => "admin",
Ok(WorkspaceRole::User) => "user",
_ => "unknown",
};
let role_str = workspace_member_role_name(member.role);
println!("{:<subject_width$} {}", member.principal_subject, role_str);
}
Ok(())
}
fn workspace_member_role_name(role: i32) -> &'static str {
use openshell_core::proto::WorkspaceRole;
match WorkspaceRole::try_from(role) {
Ok(WorkspaceRole::Admin) => "admin",
Ok(WorkspaceRole::User) => "user",
_ => "unknown",
}
}
fn workspace_member_to_json(member: &openshell_core::proto::WorkspaceMember) -> serde_json::Value {
serde_json::json!({
"subject": member.principal_subject,
"role": workspace_member_role_name(member.role),
})
}
fn workspace_phase_str(workspace: &openshell_core::proto::Workspace) -> &'static str {
use openshell_core::proto::datamodel::v1::WorkspacePhase;
let phase = workspace
@@ -6947,6 +7017,7 @@ pub async fn sandbox_policy_list(
server: &str,
name: &str,
limit: u32,
output: &str,
workspace: &str,
tls: &TlsOptions,
) -> Result<()> {
@@ -6964,6 +7035,11 @@ pub async fn sandbox_policy_list(
.into_diagnostic()?;
let revisions = resp.into_inner().revisions;
let structured = policy_revision_list_json("sandbox", Some(name), &revisions)?;
if crate::output::print_output_collection(output, &structured, Clone::clone)? {
return Ok(());
}
if revisions.is_empty() {
eprintln!("No policy history found for sandbox '{name}'");
return Ok(());
@@ -6976,6 +7052,7 @@ pub async fn sandbox_policy_list(
pub async fn sandbox_policy_list_global(
server: &str,
limit: u32,
output: &str,
workspace: &str,
tls: &TlsOptions,
) -> Result<()> {
@@ -6993,6 +7070,11 @@ pub async fn sandbox_policy_list_global(
.into_diagnostic()?;
let revisions = resp.into_inner().revisions;
let structured = policy_revision_list_json("global", None, &revisions)?;
if crate::output::print_output_collection(output, &structured, Clone::clone)? {
return Ok(());
}
if revisions.is_empty() {
eprintln!("No global policy history found");
return Ok(());
@@ -7002,6 +7084,28 @@ pub async fn sandbox_policy_list_global(
Ok(())
}
fn policy_revision_list_json(
scope: &str,
sandbox: Option<&str>,
revisions: &[openshell_core::proto::SandboxPolicyRevision],
) -> Result<Vec<serde_json::Value>> {
revisions
.iter()
.map(|revision| {
let status =
PolicyStatus::try_from(revision.status).unwrap_or(PolicyStatus::Unspecified);
policy_revision_to_json(
scope,
sandbox,
None,
revision,
status,
PolicyGetView::Metadata,
)
})
.collect()
}
fn print_policy_revision_table(revisions: &[openshell_core::proto::SandboxPolicyRevision]) {
println!(
"{:<8} {:<14} {:<12} {:<24} ERROR",
@@ -7535,16 +7639,16 @@ fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String
#[cfg(test)]
mod tests {
use super::{
PolicyGetView, ProvisioningStep, build_sandbox_resource_limits,
PolicyGetView, ProvisioningStep, attached_provider_to_json, build_sandbox_resource_limits,
dockerfile_sources_supported_for_gateway, format_endpoint, format_log_line,
format_provider_attachment_table, git_sync_files, has_main_process_result,
inferred_provider_type, parse_cli_setting_value, parse_credential_expiry_cli_value,
parse_credential_expiry_pairs, parse_credential_pairs, parse_driver_config_json,
parse_secret_material_env_pairs, policy_revision_to_json,
parse_secret_material_env_pairs, policy_revision_list_json, policy_revision_to_json,
provider_profile_allows_empty_credentials, provisioning_timeout_message,
ready_false_condition_message, refresh_status_header, refresh_status_row, resolve_from,
sandbox_should_persist, sandbox_upload_plan, service_expose_status_error,
service_url_for_gateway,
sandbox_should_persist, sandbox_upload_plan, service_endpoint_to_json,
service_expose_status_error, service_url_for_gateway, workspace_member_to_json,
};
use crate::TEST_ENV_LOCK;
use crate::commands::common::progress_step_from_metadata;
@@ -7561,12 +7665,13 @@ mod tests {
PROGRESS_STEP_STARTING_SANDBOX,
};
use openshell_core::proto::{
GetSandboxConfigResponse, GpuResourceRequirements, PolicySource, PolicyStatus, Provider,
ProviderCredentialRefresh, ProviderCredentialRefreshRecoveryAction,
CredentialHandle, GetSandboxConfigResponse, GpuResourceRequirements, PolicySource,
PolicyStatus, Provider, ProviderCredentialRefresh, ProviderCredentialRefreshRecoveryAction,
ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy,
ProviderCredentialTokenGrant, ProviderProfile, ProviderProfileCredential,
ResourceRequirements, Sandbox, SandboxCondition, SandboxPhase, SandboxPolicyRevision,
SandboxStatus, datamodel::v1::ObjectMeta,
ResourceRequirements, Sandbox, SandboxCondition, SandboxPhase, SandboxPolicy,
SandboxPolicyRevision, SandboxStatus, ServiceEndpoint, ServiceEndpointResponse,
WorkspaceMember, WorkspaceRole, datamodel::v1::ObjectMeta,
};
#[test]
@@ -7597,6 +7702,174 @@ mod tests {
);
}
#[test]
fn policy_list_json_reuses_metadata_contract() {
let load_error = "policy failed after checking café.example/非常に長いパス";
let revisions = vec![SandboxPolicyRevision {
version: 7,
policy_hash: "0123456789abcdef".to_string(),
status: PolicyStatus::Failed as i32,
load_error: load_error.to_string(),
created_at_ms: 100,
loaded_at_ms: 200,
policy: Some(SandboxPolicy::default()),
provenance: std::collections::HashMap::from([(
"source".to_string(),
"provider-composition".to_string(),
)]),
}];
let values = policy_revision_list_json("sandbox", Some("dev"), &revisions)
.expect("policy list JSON");
assert_eq!(
values[0],
serde_json::json!({
"scope": "sandbox",
"sandbox": "dev",
"version": 7,
"hash": "0123456789abcdef",
"status": "failed",
"created_at_ms": 100,
"loaded_at_ms": 200,
"load_error": load_error,
"provenance": {"source": "provider-composition"},
})
);
assert!(values[0].get("policy").is_none());
assert!(values[0].get("active_version").is_none());
let unknown = policy_revision_list_json(
"global",
None,
&[SandboxPolicyRevision {
version: 8,
status: 999,
..Default::default()
}],
)
.expect("global policy list JSON");
assert_eq!(unknown[0]["scope"], "global");
assert_eq!(unknown[0]["status"], "unspecified");
assert!(unknown[0].get("sandbox").is_none());
}
#[test]
fn attached_provider_json_is_sorted_and_secret_safe() {
let provider = Provider {
metadata: Some(ObjectMeta {
name: "github".to_string(),
..Default::default()
}),
r#type: "github".to_string(),
credentials: std::collections::HashMap::from([
("Z_TOKEN".to_string(), "inline-secret".to_string()),
("SHARED".to_string(), "shared-secret".to_string()),
]),
config: std::collections::HashMap::from([
("Z_URL".to_string(), "https://secret.example".to_string()),
("A_MODE".to_string(), "sensitive-config".to_string()),
]),
credential_expires_at_ms: std::collections::HashMap::from([(
"Z_TOKEN".to_string(),
123,
)]),
profile_workspace: "internal".to_string(),
credential_handles: std::collections::HashMap::from([
(
"A_HANDLE".to_string(),
CredentialHandle {
driver: "vault".to_string(),
handle: "opaque-secret-handle".to_string(),
metadata: std::collections::HashMap::from([(
"internal".to_string(),
"metadata-value".to_string(),
)]),
},
),
("SHARED".to_string(), CredentialHandle::default()),
]),
};
let value = attached_provider_to_json(&provider);
assert_eq!(
value,
serde_json::json!({
"name": "github",
"type": "github",
"credential_keys": ["A_HANDLE", "SHARED", "Z_TOKEN"],
"config_keys": ["A_MODE", "Z_URL"],
})
);
let serialized = value.to_string();
for secret in [
"inline-secret",
"shared-secret",
"https://secret.example",
"sensitive-config",
"opaque-secret-handle",
"metadata-value",
"internal",
"123",
] {
assert!(!serialized.contains(secret), "leaked {secret}");
}
}
#[test]
fn service_endpoint_json_has_raw_fields_and_normalized_url() {
let response = ServiceEndpointResponse {
endpoint: Some(ServiceEndpoint {
metadata: Some(ObjectMeta {
workspace: "team-a".to_string(),
..Default::default()
}),
sandbox_name: "api".to_string(),
service_name: String::new(),
target_port: 8080,
..Default::default()
}),
url: "https://api.openshell.localhost:3000/".to_string(),
};
let value = service_endpoint_to_json(&response, "https://gateway.example:17670")
.expect("service endpoint JSON");
assert_eq!(
value,
serde_json::json!({
"workspace": "team-a",
"sandbox": "api",
"service": "",
"target_port": 8080,
"url": "https://api.openshell.localhost:17670/",
})
);
assert!(service_endpoint_to_json(&ServiceEndpointResponse::default(), "unused").is_none());
}
#[test]
fn workspace_member_json_uses_stable_role_names() {
for (role, expected) in [
(WorkspaceRole::Admin as i32, "admin"),
(WorkspaceRole::User as i32, "user"),
(999, "unknown"),
] {
let value = workspace_member_to_json(&WorkspaceMember {
metadata: Some(ObjectMeta {
id: "internal-id".to_string(),
..Default::default()
}),
principal_subject: "oidc-subject".to_string(),
role,
});
assert_eq!(
value,
serde_json::json!({"subject": "oidc-subject", "role": expected})
);
assert!(!value.to_string().contains("internal-id"));
}
}
#[test]
fn parse_credential_pairs_accepts_key_value_form() {
let parsed = parse_credential_pairs(&["API_KEY=abc123".to_string()]).expect("parse");