handle sandbox sessions better

Author: pthurlow

src/main.rs

@@ -89,24 +89,34 @@ unsafe extern "C" {
     fn atexit(callback: extern "C" fn()) -> i32;
 }
 
-/// Runs once at process exit. If the CLI was authenticating through a
-/// sandbox session (env var from `sandbox run`, or on-disk session
-/// from `sandbox set <id>`), emit a footer line on stderr so the user
-/// can tell at a glance which sandbox served the call. Stderr keeps
-/// stdout clean for callers that parse JSON/YAML output.
+/// Runs once at process exit. Prints a sandbox footer on stderr when
+/// the CLI is running under an on-disk sandbox session (i.e. the user
+/// ran `hotdata sandbox set <id>` to enter it from this shell). Stays
+/// silent when the sandbox comes from `HOTDATA_SANDBOX_TOKEN` in the
+/// environment: that means we're inside a `sandbox run` child, and
+/// the parent already announced the sandbox once at spawn time.
+/// Stderr keeps stdout clean for callers parsing JSON/YAML output.
 extern "C" fn print_sandbox_footer() {
     use crossterm::style::Stylize;
 
-    let sandbox_id = sandbox_session::sandbox_token_in_use()
-        .and_then(|(_, sid)| sid)
-        .or_else(|| {
-            sandbox_session::load()
-                .map(|s| s.sandbox_id)
-                .filter(|s| !s.is_empty())
-        });
-    if let Some(sid) = sandbox_id {
-        eprintln!("{}", format!("sandbox: {sid}").dark_grey());
+    // Inside a `sandbox run` child — parent printed the banner already.
+    if sandbox_session::sandbox_token_in_use().is_some() {
+        return;
     }
+    let Some(session) = sandbox_session::load() else {
+        return;
+    };
+    if session.sandbox_id.is_empty() {
+        return;
+    }
+    eprintln!(
+        "{}",
+        format!(
+            "current sandbox: {} use 'hotdata sandbox set' to change",
+            session.sandbox_id
+        )
+        .dark_grey(),
+    );
 }
 
 fn main() {

src/sandbox.rs

@@ -261,22 +261,23 @@ pub fn run(sandbox_id: Option<&str>, workspace_id: &str, name: Option<&str>, cmd
     let api = ApiClient::new(Some(workspace_id));
 
     // Mint (or re-mint, for an existing sandbox) a sandbox-scoped JWT
-    // by hitting the auth endpoint. The same call creates a sandbox
-    // when no id is provided. Either way we end up with a fresh
-    // bundle persisted to sandbox_session.json before we spawn.
-    let resp: SandboxTokenResponse = match sandbox_id {
-        Some(id) => {
-            let path = format!("/auth/sandbox/{id}");
-            api.post(&path, &serde_json::json!({}))
-        }
+    // by dispatching on grant_type at /auth/sandbox. Either way we
+    // end up with a fresh bundle persisted to sandbox_session.json
+    // before we spawn.
+    let body = match sandbox_id {
+        Some(id) => serde_json::json!({
+            "grant_type": "existing_sandbox",
+            "sandbox_id": id,
+        }),
         None => {
-            let mut body = serde_json::json!({});
+            let mut b = serde_json::json!({});
             if let Some(n) = name {
-                body["name"] = serde_json::json!(n);
+                b["name"] = serde_json::json!(n);
             }
-            api.post("/auth/sandbox", &body)
+            b
         }
     };
+    let resp: SandboxTokenResponse = api.post("/auth/sandbox", &body);
 
     let sid = resp.sandbox_id.clone();
     let sandbox_jwt = resp.token.clone();
@@ -360,12 +361,16 @@ pub fn set(sandbox_id: Option<&str>, workspace_id: &str) {
     check_sandbox_lock();
     match sandbox_id {
         Some(id) => {
-            // Mint a sandbox-scoped JWT against this existing id. The
-            // call doubles as an existence + access check (404/403 if
-            // the user can't reach it).
+            // Mint a sandbox-scoped JWT against this existing id via
+            // the grant_type=existing_sandbox dispatch. The call
+            // doubles as an existence + access check (404/403 if the
+            // user can't reach it).
             let api = ApiClient::new(Some(workspace_id));
-            let path = format!("/auth/sandbox/{id}");
-            let resp: SandboxTokenResponse = api.post(&path, &serde_json::json!({}));
+            let body = serde_json::json!({
+                "grant_type": "existing_sandbox",
+                "sandbox_id": id,
+            });
+            let resp: SandboxTokenResponse = api.post("/auth/sandbox", &body);
             persist_sandbox_session(resp, workspace_id);
 
             if let Err(e) = config::save_sandbox("default", id) {

src/sandbox_session.rs

@@ -2,13 +2,16 @@
 //!
 //! Distinct from the user-scoped session in [`crate::jwt`]:
 //!
-//! * Minted by `/v1/auth/sandbox` (or `/v1/auth/sandbox/<id>`), not
-//!   `/o/token/`.
+//! * Minted by `POST /v1/auth/sandbox` (with no body, or
+//!   `grant_type=existing_sandbox` + `sandbox_id`), not `/o/token/`.
 //! * Bound to a single sandbox + workspace; the JWT carries only
 //!   workspace-read + sandbox-read/write scope.
-//! * Refreshed via `/v1/auth/sandbox/refresh`, which rotates the
-//!   refresh token (single-use). The user's own credentials are never
-//!   involved — possession of the sandbox refresh token is enough.
+//! * Refreshed via `POST /v1/auth/sandbox` with
+//!   `grant_type=refresh_token` — same endpoint as the new-mint path,
+//!   dispatched by body field (mirrors `POST /o/token/`). The server
+//!   does **not** rotate the refresh token. The user's own credentials
+//!   are never involved — possession of the sandbox refresh token is
+//!   enough.
 //!
 //! Stored at `~/.hotdata/sandbox_session.json` (mode 0600).
 
@@ -91,14 +94,22 @@ fn redact(s: &str) -> String {
     util::mask_credential(s)
 }
 
-/// Trade a refresh token for a fresh sandbox JWT (and a new refresh
-/// token). The server rotates: the old refresh token is dead after a
-/// successful call, so the new value must be persisted before the
-/// caller can recover from a crash.
+/// Trade a refresh token for a fresh sandbox JWT. The server does
+/// **not** rotate the refresh token (matches DOT's
+/// ``ROTATE_REFRESH_TOKEN=False``), so the same value is returned on
+/// every call. Same endpoint as the new-mint path —
+/// ``POST /v1/auth/sandbox`` with ``grant_type=refresh_token`` in the
+/// body, mirroring ``POST /o/token/``.
 pub fn refresh(api_url: &str, refresh_token: &str) -> Result<SandboxSession, String> {
-    let url = format!("{}/auth/sandbox/refresh", api_url.trim_end_matches('/'));
-    let body = serde_json::json!({"refresh_token": refresh_token});
-    let body_log = serde_json::json!({"refresh_token": redact(refresh_token)});
+    let url = format!("{}/auth/sandbox", api_url.trim_end_matches('/'));
+    let body = serde_json::json!({
+        "grant_type": "refresh_token",
+        "refresh_token": refresh_token,
+    });
+    let body_log = serde_json::json!({
+        "grant_type": "refresh_token",
+        "refresh_token": redact(refresh_token),
+    });
 
     let client = reqwest::blocking::Client::new();
     let req = client.post(&url).json(&body);
@@ -303,29 +314,36 @@ mod tests {
     }
 
     #[test]
-    fn refresh_success_rotates_tokens() {
+    fn refresh_posts_grant_type_to_sandbox_endpoint() {
         let mut server = mockito::Server::new();
         let m = server
-            .mock("POST", "/auth/sandbox/refresh")
+            .mock("POST", "/auth/sandbox")
+            .match_body(mockito::Matcher::AllOf(vec![
+                mockito::Matcher::JsonString(
+                    r#"{"grant_type":"refresh_token","refresh_token":"stable-refresh"}"#
+                        .to_string(),
+                ),
+            ]))
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(
-                r#"{"ok":true,"token":"new-jwt","refresh_token":"new-refresh","sandbox_id":"s_abc12345","expires_in":259200,"refresh_expires_in":2592000}"#,
+                // Server does not rotate — same refresh_token comes back.
+                r#"{"ok":true,"token":"new-jwt","refresh_token":"stable-refresh","sandbox_id":"s_abc12345","expires_in":300,"refresh_expires_in":259200}"#,
             )
             .create();
 
-        let s = refresh(&server.url(), "old-refresh").unwrap();
+        let s = refresh(&server.url(), "stable-refresh").unwrap();
         m.assert();
         assert_eq!(s.access_token, "new-jwt");
-        assert_eq!(s.refresh_token, "new-refresh");
+        assert_eq!(s.refresh_token, "stable-refresh");
         assert_eq!(s.sandbox_id, "s_abc12345");
     }
 
     #[test]
     fn refresh_http_error() {
         let mut server = mockito::Server::new();
         let m = server
-            .mock("POST", "/auth/sandbox/refresh")
+            .mock("POST", "/auth/sandbox")
             .with_status(401)
             .create();
         let err = refresh(&server.url(), "x").unwrap_err();
@@ -343,19 +361,20 @@ mod tests {
 
         let mut server = mockito::Server::new();
         let m = server
-            .mock("POST", "/auth/sandbox/refresh")
+            .mock("POST", "/auth/sandbox")
             .with_status(200)
             .with_header("content-type", "application/json")
             .with_body(
-                r#"{"ok":true,"token":"refreshed","refresh_token":"rotated","sandbox_id":"s_abc12345","expires_in":259200,"refresh_expires_in":2592000}"#,
+                r#"{"ok":true,"token":"refreshed","refresh_token":"cached-refresh","sandbox_id":"s_abc12345","expires_in":300,"refresh_expires_in":259200}"#,
             )
             .create();
         let tok = ensure_access_token(&server.url());
         m.assert();
         assert_eq!(tok.as_deref(), Some("refreshed"));
         let after = load().unwrap();
         assert_eq!(after.access_token, "refreshed");
-        assert_eq!(after.refresh_token, "rotated");
+        // No rotation — same refresh_token as before.
+        assert_eq!(after.refresh_token, "cached-refresh");
         assert_eq!(after.workspace_id, "work_xyz");
     }
 }

src/util.rs

@@ -284,16 +284,34 @@ pub fn format_date(s: &str) -> String {
 }
 
 pub fn api_error(body: String) -> String {
-    serde_json::from_str::<serde_json::Value>(&body)
-        .ok()
-        .and_then(|v| v["error"]["message"].as_str().map(str::to_string))
-        .unwrap_or_else(|| {
-            if body.trim_start().starts_with('<') {
-                "unexpected server error".to_string()
-            } else {
-                body
-            }
-        })
+    if let Ok(v) = serde_json::from_str::<serde_json::Value>(&body) {
+        // Two shapes in the wild:
+        //   {"error": {"message": "..."}}   — RuntimeDB-style
+        //   {"error": "snake_case_code"}    — Django-style (e.g. sandbox endpoints)
+        if let Some(m) = v["error"]["message"].as_str() {
+            return m.to_string();
+        }
+        if let Some(code) = v["error"].as_str() {
+            return humanize_error_code(code);
+        }
+    }
+    if body.trim_start().starts_with('<') {
+        return "unexpected server error".to_string();
+    }
+    body
+}
+
+/// Turn a snake_case error code into a human-friendly sentence:
+/// ``sandbox_not_found`` → ``Sandbox not found``. Cheap heuristic — if
+/// a code reads badly after this, the server should be the one to fix
+/// it by returning a real message.
+fn humanize_error_code(code: &str) -> String {
+    let spaced = code.replace('_', " ");
+    let mut chars = spaced.chars();
+    match chars.next() {
+        Some(c) => c.to_uppercase().chain(chars).collect(),
+        None => String::new(),
+    }
 }
 
 #[cfg(test)]
@@ -321,6 +339,33 @@ mod tests {
         assert_eq!(mask_credential(""), "***");
     }
 
+    #[test]
+    fn api_error_humanizes_snake_case_code() {
+        // Django-style flat shape — `sandbox_not_found` should render
+        // as a readable sentence, not a raw JSON blob.
+        let body = r#"{"error": "sandbox_not_found"}"#.to_string();
+        assert_eq!(api_error(body), "Sandbox not found");
+    }
+
+    #[test]
+    fn api_error_prefers_nested_message_over_code() {
+        // RuntimeDB-style nested shape — use the human message verbatim.
+        let body = r#"{"error": {"message": "Query qrun_x not found"}}"#.to_string();
+        assert_eq!(api_error(body), "Query qrun_x not found");
+    }
+
+    #[test]
+    fn api_error_falls_through_for_plain_body() {
+        let body = "raw text body".to_string();
+        assert_eq!(api_error(body), "raw text body");
+    }
+
+    #[test]
+    fn api_error_handles_html_body() {
+        let body = "<html>500</html>".to_string();
+        assert_eq!(api_error(body), "unexpected server error");
+    }
+
     #[test]
     fn redact_json_fields_top_level() {
         let mut v = json!({