fix(cli): resolve 6 bugs found during E2E testing (#110)

* fix(cli): resolve 6 bugs found during E2E testing

- datasets create: add missing -o/--output flag; move banner to stderr
- sandbox new: move "Sandbox created" banner to stderr so -o json is parseable
- sandbox read: use println! instead of print! to ensure trailing newline
- sandbox: add delete subcommand (DELETE /sandboxes/{id})
- workspaces set: check HOTDATA_SANDBOX instead of HOTDATA_WORKSPACE; update error message
- context push: surface friendly hint when server blocks push inside an active sandbox

* fix(sandbox): move update banner to stderr; add functional test suite

- sandbox update: move "Sandbox updated" banner to eprintln! (stderr)
  so -o json stdout is clean — same fix as sandbox new
- scripts/test_commands.sh: new comprehensive functional test script
  covering all 128 command/subcommand/flag/output-format combinations,
  with real API calls, resource lifecycle, and cleanup

* fix(sandbox): clear active session after deleting the active sandbox

When the deleted sandbox is the currently active one (tracked via
HOTDATA_SANDBOX env var or config.sandbox), clear the cached sandbox
session and config pointer so subsequent commands do not keep routing
through a stale JWT -- mirrors what sandbox set (no args) already does.

* fix(sandbox): block delete inside an active sandbox run

Add check_sandbox_lock() to sandbox::delete for consistency with new,
run, and set -- prevents silently deleting the sandbox you are currently
running inside.

* revert: remove test script from PR

---------

Co-authored-by: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com>

Author: eddietejeda

src/command.rs

@@ -478,6 +478,10 @@ pub enum DatasetsCommands {
         /// Saved query ID to create the dataset from
         #[arg(long, conflicts_with = "sql", required_unless_present = "sql")]
         query_id: Option<String>,
+
+        /// Output format
+        #[arg(long = "output", short = 'o', default_value = "table", value_parser = ["table", "json", "yaml"])]
+        output: String,
     },
 
     /// Update a dataset's description and/or name
@@ -854,6 +858,12 @@ pub enum SandboxCommands {
         #[arg(trailing_var_arg = true, required = true)]
         cmd: Vec<String>,
     },
+
+    /// Delete a sandbox permanently
+    Delete {
+        /// Sandbox ID to delete
+        id: String,
+    },
 }
 
 #[derive(Subcommand)]

src/context.rs

@@ -311,7 +311,28 @@ pub fn push(workspace_id: &str, database_id: &str, name: &str, dry_run: bool) {
 
     let api = ApiClient::new(Some(workspace_id));
     let body = json!({ "name": &name, "content": content });
-    let resp: UpsertResponse = api.post(&format!("/databases/{database_id}/context"), &body);
+    let (status, resp_body) = api.post_raw(&format!("/databases/{database_id}/context"), &body);
+
+    if !status.is_success() {
+        let msg = crate::util::api_error(resp_body.clone());
+        if msg.to_lowercase().contains("not allowed within a session") {
+            eprintln!("{}", msg.red());
+            eprintln!(
+                "{}",
+                "hint: context push is blocked inside an active sandbox. \
+Run 'hotdata sandbox set' (no args) to clear the active sandbox first."
+                    .dark_grey()
+            );
+        } else {
+            eprintln!("{}", msg.red());
+        }
+        std::process::exit(1);
+    }
+
+    let resp: UpsertResponse = serde_json::from_str(&resp_body).unwrap_or_else(|e| {
+        eprintln!("error parsing response: {e}");
+        std::process::exit(1);
+    });
 
     println!(
         "{}",

src/datasets.rs

@@ -17,7 +17,7 @@ fn default_schema() -> String {
     "main".to_string()
 }
 
-#[derive(Deserialize)]
+#[derive(Deserialize, Serialize)]
 struct CreateResponse {
     id: String,
     label: String,
@@ -75,6 +75,7 @@ fn create_dataset(
     description: Option<&str>,
     name: &str,
     source: serde_json::Value,
+    format: &str,
 ) {
     let label = description.unwrap_or(name);
     let body = json!({ "table_name": name, "label": label, "source": source });
@@ -96,28 +97,36 @@ fn create_dataset(
     };
 
     use crossterm::style::Stylize;
-    println!("{}", "Dataset created".green());
-    println!("id:         {}", dataset.id);
-    println!("label:      {}", dataset.label);
-    println!(
-        "full_name:  datasets.{}.{}",
-        dataset.schema_name, dataset.table_name
-    );
+    match format {
+        "json" => println!("{}", serde_json::to_string_pretty(&dataset).unwrap()),
+        "yaml" => print!("{}", serde_yaml::to_string(&dataset).unwrap()),
+        "table" => {
+            eprintln!("{}", "Dataset created".green());
+            println!("id:         {}", dataset.id);
+            println!("label:      {}", dataset.label);
+            println!(
+                "full_name:  datasets.{}.{}",
+                dataset.schema_name, dataset.table_name
+            );
+        }
+        _ => unreachable!(),
+    }
 }
 
-pub fn create_from_query(workspace_id: &str, sql: &str, description: Option<&str>, name: &str) {
+pub fn create_from_query(workspace_id: &str, sql: &str, description: Option<&str>, name: &str, format: &str) {
     let api = ApiClient::new(Some(workspace_id));
-    create_dataset(&api, description, name, json!({ "type": "sql_query", "sql": sql }));
+    create_dataset(&api, description, name, json!({ "type": "sql_query", "sql": sql }), format);
 }
 
 pub fn create_from_saved_query(
     workspace_id: &str,
     query_id: &str,
     description: Option<&str>,
     name: &str,
+    format: &str,
 ) {
     let api = ApiClient::new(Some(workspace_id));
-    create_dataset(&api, description, name, json!({ "type": "saved_query", "saved_query_id": query_id }));
+    create_dataset(&api, description, name, json!({ "type": "saved_query", "saved_query_id": query_id }), format);
 }
 
 pub fn list(workspace_id: &str, limit: Option<u32>, offset: Option<u32>, format: &str) {

src/main.rs

@@ -216,20 +216,23 @@ fn main() {
                             description,
                             sql,
                             query_id,
+                            output,
                         }) => {
                             if let Some(sql) = sql {
                                 datasets::create_from_query(
                                     &workspace_id,
                                     &sql,
                                     description.as_deref(),
                                     &name,
+                                    &output,
                                 )
                             } else {
                                 datasets::create_from_saved_query(
                                     &workspace_id,
                                     query_id.as_deref().unwrap_or_else(|| unreachable!("clap enforces --sql or --query-id")),
                                     description.as_deref(),
                                     &name,
+                                    &output,
                                 )
                             }
                         }
@@ -957,6 +960,9 @@ fn main() {
                     Some(SandboxCommands::Set { id: set_id }) => {
                         sandbox::set(set_id.as_deref(), &workspace_id)
                     }
+                    Some(SandboxCommands::Delete { id: delete_id }) => {
+                        sandbox::delete(&delete_id, &workspace_id)
+                    }
                     None => match id {
                         Some(id) => sandbox::get(&id, &workspace_id, &output),
                         None => {

src/sandbox.rs

@@ -135,7 +135,7 @@ pub fn read(sandbox_id: &str, workspace_id: &str) {
     if body.sandbox.markdown.is_empty() {
         eprintln!("{}", "Sandbox markdown is empty.".dark_grey());
     } else {
-        print!("{}", body.sandbox.markdown);
+        println!("{}", body.sandbox.markdown);
     }
 }
 
@@ -195,7 +195,7 @@ pub fn new(workspace_id: &str, name: Option<&str>, format: &str) {
         eprintln!("warning: could not save sandbox to config: {e}");
     }
 
-    println!("{}", "Sandbox created".green());
+    eprintln!("{}", "Sandbox created".green());
     match format {
         "json" => println!("{}", serde_json::json!({"public_id": sandbox_id})),
         "yaml" => print!(
@@ -238,7 +238,7 @@ pub fn update(
     let resp: DetailResponse = api.patch(&path, &body);
     let s = &resp.sandbox;
 
-    println!("{}", "Sandbox updated".green());
+    eprintln!("{}", "Sandbox updated".green());
     match format {
         "json" => println!("{}", serde_json::to_string_pretty(s).unwrap()),
         "yaml" => print!("{}", serde_yaml::to_string(s).unwrap()),
@@ -340,6 +340,33 @@ pub fn set(sandbox_id: Option<&str>, workspace_id: &str) {
     }
 }
 
+pub fn delete(sandbox_id: &str, workspace_id: &str) {
+    check_sandbox_lock();
+    let api = ApiClient::new(Some(workspace_id));
+    let path = format!("/sandboxes/{sandbox_id}");
+    let (status, resp_body) = api.delete_raw(&path);
+
+    if !status.is_success() {
+        eprintln!("{}", crate::util::api_error(resp_body).red());
+        std::process::exit(1);
+    }
+
+    // If the deleted sandbox was the active one, clear the cached session
+    // and config pointer so subsequent commands don't keep routing through
+    // a stale sandbox JWT — mirroring what `sandbox set` (no args) does.
+    let active = std::env::var("HOTDATA_SANDBOX")
+        .ok()
+        .or_else(|| config::load("default").ok().and_then(|p| p.sandbox));
+    if active.as_deref() == Some(sandbox_id) {
+        sandbox_session::clear();
+        if let Err(e) = config::clear_sandbox("default") {
+            eprintln!("warning: could not clear sandbox from config: {e}");
+        }
+    }
+
+    eprintln!("{}", "Sandbox deleted".green());
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/workspace.rs

@@ -17,10 +17,10 @@ struct ListResponse {
 }
 
 pub fn set(workspace_id: Option<&str>) {
-    if std::env::var("HOTDATA_WORKSPACE").is_ok()
+    if std::env::var("HOTDATA_SANDBOX").is_ok()
         || crate::sandbox::find_sandbox_run_ancestor().is_some()
     {
-        eprintln!("error: workspace is locked");
+        eprintln!("error: workspace cannot be changed inside a sandbox");
         std::process::exit(1);
     }
     let api = ApiClient::new(None);