diff --git a/crates/openshell-cli/src/commands/common.rs b/crates/openshell-cli/src/commands/common.rs index 316e590925..1b02447b3f 100644 --- a/crates/openshell-cli/src/commands/common.rs +++ b/crates/openshell-cli/src/commands/common.rs @@ -1113,6 +1113,27 @@ pub fn scrub_git_env(command: &mut Command) -> &mut Command { mod tests { use super::*; + #[test] + fn parse_key_value_pairs_keeps_slashes_spaces_and_equals() { + let items = vec!["source=/home/you/my policy.yaml?a=b".to_string()]; + let map = parse_key_value_pairs(&items, "--annotation").expect("annotation should parse"); + assert_eq!( + map.get("source").map(String::as_str), + Some("/home/you/my policy.yaml?a=b") + ); + } + + #[test] + fn parse_key_value_pairs_rejects_malformed_and_empty_keys() { + let err = + parse_key_value_pairs(&["nokey".to_string()], "--annotation").expect_err("missing '='"); + assert!(err.to_string().contains("--annotation expects KEY=VALUE")); + + let err = + parse_key_value_pairs(&["=value".to_string()], "--annotation").expect_err("empty key"); + assert!(err.to_string().contains("--annotation key cannot be empty")); + } + #[test] fn parse_duration_to_ms_parses_supported_units() { assert_eq!(parse_duration_to_ms("30s").expect("parse"), 30_000); diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 5f20fc7f3c..4e762c2d1d 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -10,7 +10,6 @@ use clap_complete::engine::ArgValueCompleter; use clap_complete::env::CompleteEnv; use miette::Result; use openshell_cli::color::{self, ColorChoice, Colorize}; -use std::collections::HashMap; use std::io::Write; use std::path::PathBuf; @@ -1590,6 +1589,11 @@ enum SandboxCommands { #[arg(long = "label")] labels: Vec, + /// Attach non-secret annotations to the sandbox (key=value format, + /// repeatable). Values may contain `/`, spaces, and `=`. + #[arg(long = "annotation", value_name = "KEY=VALUE")] + annotations: Vec, + /// Set a non-secret environment variable in the sandbox. /// Do not use this option for API keys, tokens, or other secrets; create /// a provider and attach it with `--provider` instead. Repeatable. @@ -3378,6 +3382,7 @@ async fn run_async() -> Result<()> { auto_providers, no_auto_providers, labels, + annotations, envs, env_from, no_credential_warnings, @@ -3403,18 +3408,9 @@ async fn run_async() -> Result<()> { None // prompt or auto-detect }; - // Parse --label flags into a HashMap. - let mut labels_map = HashMap::new(); - for label_str in &labels { - let parts: Vec<&str> = label_str.splitn(2, '=').collect(); - if parts.len() != 2 { - return Err(miette::miette!( - "invalid label format '{}', expected key=value", - label_str - )); - } - labels_map.insert(parts[0].to_string(), parts[1].to_string()); - } + // Parse --label and --annotation flags into maps. + let labels_map = run::parse_key_value_pairs(&labels, "--label")?; + let annotations_map = run::parse_key_value_pairs(&annotations, "--annotation")?; // Parse --env flags into a HashMap. let mut env_map = run::parse_env_pairs(&envs)?; @@ -3471,6 +3467,7 @@ async fn run_async() -> Result<()> { tty_override, auto_providers_override, labels: labels_map, + annotations: annotations_map, environment: env_map, approval_mode: &approval_mode, output: output.as_str(), @@ -6208,6 +6205,33 @@ mod tests { } } + #[test] + fn sandbox_create_annotation_is_repeatable() { + let cli = Cli::try_parse_from([ + "openshell", + "sandbox", + "create", + "--annotation", + "policy-source-file=/home/you/my policy.yaml", + "--annotation", + "owner=platform", + ]) + .unwrap(); + match cli.command { + Some(Commands::Sandbox { + command: Some(SandboxCommands::Create { annotations, .. }), + .. + }) => assert_eq!( + annotations, + vec![ + "policy-source-file=/home/you/my policy.yaml".to_string(), + "owner=platform".to_string(), + ] + ), + other => panic!("expected SandboxCommands::Create, got: {other:?}"), + } + } + #[test] fn sandbox_create_restart_policy_defaults_to_never() { let cli = Cli::try_parse_from(["openshell", "sandbox", "create"]).unwrap(); diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 807a79cb72..cb42626a6f 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -450,6 +450,7 @@ pub struct SandboxCreateConfig<'a> { pub tty_override: Option, pub auto_providers_override: Option, pub labels: HashMap, + pub annotations: HashMap, pub environment: HashMap, pub approval_mode: &'a str, pub output: &'a str, @@ -480,6 +481,7 @@ impl Default for SandboxCreateConfig<'_> { tty_override: None, auto_providers_override: None, labels: HashMap::new(), + annotations: HashMap::new(), environment: HashMap::new(), approval_mode: "manual", output: "table", @@ -518,6 +520,7 @@ pub async fn sandbox_create( tty_override, auto_providers_override, labels, + mut annotations, environment, approval_mode, output, @@ -662,14 +665,12 @@ pub async fn sandbox_create( && command.is_empty() && (!std::io::stdin().is_terminal() || !std::io::stdout().is_terminal())); let await_main_process_attachment = output == "table" && editor.is_none() && !create_detaches; - let annotations = if persist { - HashMap::new() - } else { - HashMap::from([( + if !persist { + annotations.insert( "openshell.nvidia.com/retention".to_string(), "ephemeral".to_string(), - )]) - }; + ); + } let request = CreateSandboxRequest { request_id: String::new(), spec: Some(SandboxSpec { diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 625d944fa4..31345d6e2e 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -2035,6 +2035,97 @@ async fn detached_ephemeral_command_delegates_cleanup_to_gateway() { .await; } +#[tokio::test] +async fn sandbox_create_sends_annotations() { + let server = run_server().await; + let fake_ssh_dir = helpers::tempdir().unwrap(); + let xdg_dir = helpers::tempdir().unwrap(); + + test_env(&fake_ssh_dir, &xdg_dir) + .run_async(async { + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + let source = "/home/you/my policy.yaml=1".to_string(); + run::sandbox_create( + &server.endpoint, + "openshell", + run::SandboxCreateConfig { + name: Some("annotated"), + keep: true, + command: &["echo".into(), "OK".into()], + annotations: HashMap::from([( + "policy-source-file".to_string(), + source.clone(), + )]), + ..test_config() + }, + "default", + &tls, + ) + .await + .expect("sandbox create should succeed"); + + let requests = create_requests(&server).await; + assert_eq!( + requests[0] + .annotations + .get("policy-source-file") + .map(String::as_str), + Some(source.as_str()) + ); + assert!( + !requests[0] + .annotations + .contains_key("openshell.nvidia.com/retention"), + "a kept sandbox must not be marked ephemeral" + ); + }) + .await; +} + +#[tokio::test] +async fn ephemeral_sandbox_create_keeps_annotations_and_retention() { + let server = run_server().await; + let fake_ssh_dir = helpers::tempdir().unwrap(); + let xdg_dir = helpers::tempdir().unwrap(); + + test_env(&fake_ssh_dir, &xdg_dir) + .run_async(async { + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + run::sandbox_create( + &server.endpoint, + "openshell", + run::SandboxCreateConfig { + name: Some("annotated-ephemeral"), + keep: false, + command: &["echo".into(), "OK".into()], + annotations: HashMap::from([("owner".to_string(), "platform".to_string())]), + ..test_config() + }, + "default", + &tls, + ) + .await + .expect("sandbox create should succeed"); + + let annotations = &create_requests(&server).await[0].annotations; + assert_eq!( + annotations.get("owner").map(String::as_str), + Some("platform") + ); + assert_eq!( + annotations + .get("openshell.nvidia.com/retention") + .map(String::as_str), + Some("ephemeral") + ); + }) + .await; +} + #[tokio::test] async fn sandbox_create_sends_driver_config_json() { let server = run_server().await; @@ -2992,7 +3083,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() { let tls = test_tls(&server); install_fake_forwarding_ssh(&fake_ssh_dir); let (endpoint, tls) = (server.endpoint.as_str(), &tls); - let (forward_port, result) = with_free_forward_port(|spec| async move { + let (forward_port, result) = Box::pin(with_free_forward_port(|spec| async move { run::sandbox_create( endpoint, "openshell", @@ -3007,7 +3098,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() { tls, ) .await - }) + })) .await; result.expect("sandbox create with forward should succeed"); diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index f10cccb561..b637ca21ff 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -437,6 +437,12 @@ openshell sandbox list --selector env=dev openshell sandbox list --selector env=dev,team=platform ``` +Use `--annotation KEY=VALUE` for non-secret metadata that does not fit label limits, such as a file path. Annotation values can contain `/`, spaces, and `=`, and appear in the `annotations` field of `sandbox get -o json` and `sandbox list -o json`. + +```shell +openshell sandbox create --annotation "policy-source-file=/home/you/policies/research policy.yaml" -- claude +``` + The Python SDK accepts the same gateway labels and selectors. Labels passed to `create` are stored on the gateway sandbox object and are returned on `SandboxRef.labels`, so selector-based listing finds Python-created sandboxes: diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 75d2de5057..8fe37fee1c 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -305,6 +305,7 @@ Key flags: - `--driver-config-json`: Pass experimental driver-specific sandbox configuration - `--template NAME`: Create from a named sandbox workload template. Conflicts with inline workload flags such as `--from`, `--gpu`, `--cpu`, `--memory`, `--env`, `--env-from`, and `--driver-config-json`. - `--label KEY=VALUE`: Add labels for later selection (repeatable) +- `--annotation KEY=VALUE`: Add non-secret metadata such as file paths (repeatable) - `--env KEY=VALUE`: Set non-secret sandbox environment variables (repeatable); use `--provider` for credentials - `--env-from KEY[=ENVVAR]`: Read a sandbox environment value from the CLI process environment without putting its value in arguments; use `--provider` to keep credentials hidden from the workload - `--tty`: Allocate a retained PTY for the canonical main process