fix: address vertex review findings

Author: maxamillion

crates/openshell-cli/src/main.rs

@@ -184,7 +184,7 @@ fn resolve_sandbox_name(name: Option<String>, gateway: &str) -> Result<String> {
     let last = load_last_sandbox(gateway).ok_or_else(|| {
         miette::miette!(
             "No sandbox name provided and no last-used sandbox.\n\
-             Specify a sandbox name or connect to one first: nav sandbox connect <name>"
+             Specify a sandbox name or connect to one first: openshell sandbox connect <name>"
         )
     })?;
     eprintln!("{} Using sandbox '{}' (last used)", "→".bold(), last.bold());
@@ -3490,7 +3490,7 @@ mod tests {
             let err = resolve_sandbox_name(None, "unknown-gateway").unwrap_err();
             let msg = err.to_string();
             assert!(
-                msg.contains("nav sandbox connect"),
+                msg.contains("openshell sandbox connect"),
                 "expected helpful hint in error, got: {msg}"
             );
         });

crates/openshell-cli/src/run.rs

@@ -60,7 +60,7 @@ use openshell_providers::{
 };
 use owo_colors::OwoColorize;
 use std::collections::{HashMap, HashSet};
-use std::io::{IsTerminal, Read, Write};
+use std::io::{ErrorKind, IsTerminal, Read, Write};
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::time::{Duration, Instant};
@@ -76,6 +76,15 @@ pub use openshell_core::forward::{
     find_forward_by_port, list_forwards, stop_forward, stop_forwards_for_sandbox,
 };
 
+#[derive(Debug, PartialEq, Eq)]
+enum SandboxUploadPlan {
+    GitAware {
+        base_dir: PathBuf,
+        files: Vec<String>,
+    },
+    Regular,
+}
+
 /// Convert a sandbox phase integer to a human-readable string.
 fn phase_name(phase: i32) -> &'static str {
     match SandboxPhase::try_from(phase) {
@@ -2007,26 +2016,29 @@ pub async fn sandbox_create(
                     "\u{2022}".dimmed(),
                 );
                 let local = Path::new(local_path);
-                if *git_ignore && let Ok((base_dir, files)) = git_sync_files(local) {
-                    sandbox_sync_up_files(
-                        &effective_server,
-                        &sandbox_name,
-                        &base_dir,
-                        &files,
-                        local,
-                        dest,
-                        &effective_tls,
-                    )
-                    .await?;
-                } else if local.exists() {
-                    sandbox_sync_up(
-                        &effective_server,
-                        &sandbox_name,
-                        local,
-                        dest,
-                        &effective_tls,
-                    )
-                    .await?;
+                match sandbox_upload_plan(local, *git_ignore)? {
+                    SandboxUploadPlan::GitAware { base_dir, files } => {
+                        sandbox_sync_up_files(
+                            &effective_server,
+                            &sandbox_name,
+                            &base_dir,
+                            &files,
+                            local,
+                            dest,
+                            &effective_tls,
+                        )
+                        .await?;
+                    }
+                    SandboxUploadPlan::Regular => {
+                        sandbox_sync_up(
+                            &effective_server,
+                            &sandbox_name,
+                            local,
+                            dest,
+                            &effective_tls,
+                        )
+                        .await?;
+                    }
                 }
                 eprintln!("  {} Files uploaded", "\u{2713}".green().bold());
             }
@@ -5616,6 +5628,28 @@ pub fn git_sync_files(local_path: &Path) -> Result<(PathBuf, Vec<String>)> {
     Ok((base_dir, files))
 }
 
+fn sandbox_upload_plan(local_path: &Path, git_ignore: bool) -> Result<SandboxUploadPlan> {
+    let metadata = std::fs::symlink_metadata(local_path).map_err(|err| {
+        if err.kind() == ErrorKind::NotFound {
+            miette::miette!("local path does not exist: {}", local_path.display())
+        } else {
+            miette::miette!(
+                "failed to inspect local upload path: {}",
+                local_path.display()
+            )
+        }
+    })?;
+
+    if git_ignore
+        && !metadata.file_type().is_symlink()
+        && let Ok((base_dir, files)) = git_sync_files(local_path)
+    {
+        return Ok(SandboxUploadPlan::GitAware { base_dir, files });
+    }
+
+    Ok(SandboxUploadPlan::Regular)
+}
+
 fn scrub_git_env(command: &mut Command) -> &mut Command {
     for key in [
         "GIT_DIR",
@@ -7191,7 +7225,8 @@ mod tests {
         plaintext_gateway_is_remote, progress_step_from_metadata,
         provider_profile_allows_refresh_bootstrap, provisioning_timeout_message,
         ready_false_condition_message, refresh_status_header, refresh_status_row, resolve_from,
-        sandbox_should_persist, service_expose_status_error, service_url_for_gateway,
+        sandbox_should_persist, sandbox_upload_plan, service_expose_status_error,
+        service_url_for_gateway,
     };
     use crate::TEST_ENV_LOCK;
     use hyper::StatusCode;
@@ -7936,6 +7971,51 @@ mod tests {
         assert_eq!(files, vec!["file.txt", "inner/child.txt"]);
     }
 
+    #[test]
+    fn sandbox_upload_plan_errors_for_missing_local_path() {
+        let tmpdir = tempfile::tempdir().expect("create tmpdir");
+        let missing = tmpdir.path().join("missing");
+
+        let err = sandbox_upload_plan(&missing, false).expect_err("missing path should error");
+
+        assert!(
+            err.to_string().contains("local path does not exist"),
+            "expected missing-path error, got: {err}"
+        );
+    }
+
+    #[test]
+    fn sandbox_upload_plan_errors_for_missing_local_path_with_git_ignore() {
+        let tmpdir = tempfile::tempdir().expect("create tmpdir");
+        let repo = tmpdir.path().join("repo");
+        fs::create_dir_all(&repo).expect("create repo");
+        init_git_repo(&repo);
+        let missing = repo.join("missing");
+
+        let err = sandbox_upload_plan(&missing, true).expect_err("missing path should error");
+
+        assert!(
+            err.to_string().contains("local path does not exist"),
+            "expected missing-path error, got: {err}"
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn sandbox_upload_plan_uses_regular_upload_for_symlinks() {
+        let tmpdir = tempfile::tempdir().expect("create tmpdir");
+        let repo = tmpdir.path().join("repo");
+        fs::create_dir_all(repo.join("real-dir")).expect("create repo");
+        init_git_repo(&repo);
+        fs::write(repo.join("real-dir/file.txt"), "file").expect("write file.txt");
+        std::os::unix::fs::symlink("real-dir", repo.join("link-dir")).expect("create symlink");
+
+        let plan = sandbox_upload_plan(&repo.join("link-dir"), true)
+            .expect("symlink upload should be planned");
+
+        assert_eq!(plan, super::SandboxUploadPlan::Regular);
+    }
+
     #[test]
     fn git_sync_files_ignores_inherited_git_env() {
         let _lock = TEST_ENV_LOCK

crates/openshell-router/src/backend.rs

@@ -213,38 +213,43 @@ fn prepare_backend_request(
     //   path) and inject "anthropic_version" (required in the body, not a header).
     // Non-JSON bodies pass through unchanged; model rewrite and version injection
     // are silently skipped. Such bodies would be rejected by the upstream anyway.
-    let body = serde_json::from_slice::<serde_json::Value>(&body).map_or(body, |mut json| {
-        if let Some(obj) = json.as_object_mut() {
-            // Vertex AI Anthropic endpoints require anthropic_version in the body.
-            // Standard Anthropic SDK sends it as a header; Vertex AI needs it as a body field.
-            // We inject it only for the Vertex rawPredict-style route contract used for
-            // Anthropic publisher endpoints, not for arbitrary model-in-path routes.
-            let needs_vertex_anthropic_version = is_vertex_anthropic_rawpredict_route(route);
-            if needs_vertex_anthropic_version {
-                // Vertex AI rawPredict encodes the model in the URL path, not
-                // the request body. Clients using the standard Anthropic API
-                // (e.g. Claude Code via inference.local) always send "model"
-                // in the body; strip it so Vertex AI does not reject the
-                // request with "Extra inputs are not permitted".
-                obj.remove("model");
-            } else {
-                obj.insert(
-                    "model".to_string(),
-                    serde_json::Value::String(route.model.clone()),
-                );
-            }
-            if needs_vertex_anthropic_version && !obj.contains_key("anthropic_version") {
-                obj.insert(
-                    "anthropic_version".to_string(),
-                    serde_json::Value::String(VERTEX_ANTHROPIC_VERSION.to_string()),
-                );
+    let body = match serde_json::from_slice::<serde_json::Value>(&body) {
+        Ok(mut json) => {
+            if let Some(obj) = json.as_object_mut() {
+                // Vertex AI Anthropic endpoints require anthropic_version in the body.
+                // Standard Anthropic SDK sends it as a header; Vertex AI needs it as a body field.
+                // We inject it only for the Vertex rawPredict-style route contract used for
+                // Anthropic publisher endpoints, not for arbitrary model-in-path routes.
+                let needs_vertex_anthropic_version = is_vertex_anthropic_rawpredict_route(route);
+                if needs_vertex_anthropic_version {
+                    // Vertex AI rawPredict encodes the model in the URL path, not
+                    // the request body. Clients using the standard Anthropic API
+                    // (e.g. Claude Code via inference.local) always send "model"
+                    // in the body; strip it so Vertex AI does not reject the
+                    // request with "Extra inputs are not permitted".
+                    obj.remove("model");
+                } else {
+                    obj.insert(
+                        "model".to_string(),
+                        serde_json::Value::String(route.model.clone()),
+                    );
+                }
+                if needs_vertex_anthropic_version && !obj.contains_key("anthropic_version") {
+                    obj.insert(
+                        "anthropic_version".to_string(),
+                        serde_json::Value::String(VERTEX_ANTHROPIC_VERSION.to_string()),
+                    );
+                }
             }
+
+            bytes::Bytes::from(serde_json::to_vec(&json).map_err(|err| {
+                RouterError::Internal(format!(
+                    "failed to serialize rewritten inference request body: {err}"
+                ))
+            })?)
         }
-        bytes::Bytes::from(
-            serde_json::to_vec(&json)
-                .expect("re-serializing a valid serde_json::Value cannot fail"),
-        )
-    });
+        Err(_) => body,
+    };
     builder = builder.body(body);
 
     Ok((builder, url))