fix(runtime-api): review follow-ups for workspace, git and job paths

- Jobs: resolve `cwd` with tokio::fs so no path resolution runs on a
  runtime worker (the #6149 blocking-call ratchet failed on the two inline
  canonicalize calls). Containment is still checked on the canonical path,
  but the manager gets the joined spelling as before, so Windows gets no
  `\\?\` verbatim cwd. Doc comment and RUNTIME_API.md now say the route is
  stricter than the shell tool's resolve_path (no follow_symlinks, trusted
  roots or `~`), so a symlink leading out of the workspace is refused.
- `.git` rule: one `snapshot::is_git_metadata_name` for workspace routes,
  file restore (validate_restore_file had its own ASCII-case-only check),
  snapshot delta display paths, turn-artifact display paths and the LSP
  semantic paths. The Windows forms live in a pure helper that takes the
  platform as a bool, so trailing dot/space, `:stream` and `GIT~N` are
  tested on every host.
- Push: `push_args` builds the arguments, validates the effective remote
  (including the `origin` default for set_upstream) and puts `--` before
  the remote.

Tests: 3 new/extended tests. Targeted runtime_api::{jobs,git,workspace},
snapshot::{repo,delta}, turn_artifacts and lsp filters: 132 passed /
0 failed. With the three fixes reverted: git_metadata_name, job_cwd and
push_args tests fail (1 passed / 3 failed). cargo fmt --check clean;
check-blocking-calls-budget, check-runtime-contract-budget and
check-dead-code-budget pass; clippy reports nothing in touched files.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_014ZwqatxgVFxHvovngywnks

crates/tui/src/lsp/mod.rs

@@ -743,7 +743,7 @@ impl LspManager {
             };
             let relative = match target.strip_prefix(&self.workspace).or_else(|_| target.strip_prefix(&root)) {
                 Ok(relative) if !relative.as_os_str().is_empty()
-                    && relative.components().all(|component| matches!(component, std::path::Component::Normal(part) if part != ".git")) => relative,
+                    && relative.components().all(|component| matches!(component, std::path::Component::Normal(part) if !crate::snapshot::is_git_metadata_name(part))) => relative,
                 _ => { omitted += 1; continue; }
             };
             let Some(path) = semantic_relative_path(relative) else {
@@ -902,7 +902,7 @@ fn semantic_relative_path(path: &Path) -> Option<String> {
     let parts = path
         .components()
         .map(|component| match component {
-            std::path::Component::Normal(part) if part != ".git" => {
+            std::path::Component::Normal(part) if !crate::snapshot::is_git_metadata_name(part) => {
                 part.to_str().filter(|part| !part.contains('\\'))
             }
             _ => None,

crates/tui/src/runtime_api/git.rs

@@ -1862,24 +1862,27 @@ fn validate_push_remote(remote: &str, configured: &str) -> Result<(), ApiError>
     Ok(())
 }
 
-pub(super) async fn git_push(
-    State(state): State<RuntimeApiState>,
-    Json(request): Json<GitPushRequest>,
-) -> Result<Json<Value>, ApiError> {
-    let workspace = canonical_workspace(&state.workspace)?;
-    require_repo(&workspace)?;
-    let remote = request
-        .remote
-        .as_deref()
-        .map(str::trim)
-        .filter(|remote| !remote.is_empty())
-        .map(str::to_string);
-    if let Some(remote) = &remote {
-        validate_push_remote(remote, &run_git_sync(&workspace, &["remote"])?)?;
-    }
+/// `git push` arguments for a request. The remote that will be pushed to —
+/// the requested one, or `origin` when only `set_upstream` is asked — must be
+/// a configured remote, and `--` keeps it from ever parsing as an option.
+fn push_args(
+    workspace: &FsPath,
+    remote: Option<&str>,
+    set_upstream: bool,
+) -> Result<Vec<String>, ApiError> {
+    let remote = remote.map(str::trim).filter(|remote| !remote.is_empty());
+    let remote = match (remote, set_upstream) {
+        (Some(remote), _) => Some(remote),
+        (None, true) => Some("origin"),
+        (None, false) => None,
+    };
     let mut args = vec!["push".to_string()];
-    if request.set_upstream {
-        let branch = run_git_sync(&workspace, &["rev-parse", "--abbrev-ref", "HEAD"])?;
+    let Some(remote) = remote else {
+        return Ok(args);
+    };
+    validate_push_remote(remote, &run_git_sync(workspace, &["remote"])?)?;
+    if set_upstream {
+        let branch = run_git_sync(workspace, &["rev-parse", "--abbrev-ref", "HEAD"])?;
         let branch = branch.trim();
         if branch.is_empty() || branch == "HEAD" {
             return Err(ApiError::bad_request(
@@ -1887,11 +1890,20 @@ pub(super) async fn git_push(
             ));
         }
         args.push("--set-upstream".to_string());
-        args.push(remote.unwrap_or_else(|| "origin".to_string()));
-        args.push(branch.to_string());
-    } else if let Some(remote) = remote {
-        args.push(remote);
+        args.extend(["--".to_string(), remote.to_string(), branch.to_string()]);
+    } else {
+        args.extend(["--".to_string(), remote.to_string()]);
     }
+    Ok(args)
+}
+
+pub(super) async fn git_push(
+    State(state): State<RuntimeApiState>,
+    Json(request): Json<GitPushRequest>,
+) -> Result<Json<Value>, ApiError> {
+    let workspace = canonical_workspace(&state.workspace)?;
+    require_repo(&workspace)?;
+    let args = push_args(&workspace, request.remote.as_deref(), request.set_upstream)?;
     // Push stays outside the write lock: it crosses the network under a
     // 120 s bound and only moves a remote ref, never the index or worktree.
     mutation_response(&workspace, git_write(&workspace, args).await?).await
@@ -2694,6 +2706,50 @@ mod tests {
         }
     }
 
+    #[test]
+    fn push_args_validate_the_effective_remote_and_end_options() {
+        let tmp = repo();
+        let root = tmp.path();
+        let status = |error: ApiError| error.status;
+
+        // No remote configured: plain `push` stays git's call, but an
+        // upstream push defaults to `origin`, which must exist too.
+        assert_eq!(push_args(root, None, false).unwrap(), ["push"]);
+        assert_eq!(push_args(root, Some("  "), false).unwrap(), ["push"]);
+        assert_eq!(
+            push_args(root, None, true).map_err(status).unwrap_err(),
+            StatusCode::BAD_REQUEST
+        );
+
+        let bare = tempfile::tempdir().unwrap();
+        git(bare.path(), &["init", "-q", "--bare"]);
+        git(
+            root,
+            &["remote", "add", "origin", &bare.path().to_string_lossy()],
+        );
+        assert_eq!(
+            push_args(root, Some(" origin "), false).unwrap(),
+            ["push", "--", "origin"]
+        );
+        for remote in ["--mirror", "fork", "../other"] {
+            assert_eq!(
+                push_args(root, Some(remote), true)
+                    .map_err(status)
+                    .unwrap_err(),
+                StatusCode::BAD_REQUEST,
+                "{remote}"
+            );
+        }
+        let args = push_args(root, None, true).unwrap();
+        assert_eq!(args, ["push", "--set-upstream", "--", "origin", "main"]);
+
+        // The built arguments are ones git accepts: the push lands in the
+        // bare remote and records the upstream.
+        git(root, &args.iter().map(String::as_str).collect::<Vec<_>>());
+        let upstream = run_git_sync(root, &["rev-parse", "--abbrev-ref", "@{upstream}"]).unwrap();
+        assert_eq!(upstream.trim(), "origin/main");
+    }
+
     #[test]
     fn push_remote_must_be_a_configured_remote_name() {
         let configured = "origin\nupstream\n";

crates/tui/src/runtime_api/jobs.rs

@@ -177,10 +177,17 @@ pub(super) struct CreateJobResponse {
     job: JobView,
 }
 
-/// Resolve a job `cwd` the way the shell tool resolves its own: relative to
-/// the thread workspace (never the server's cwd) and, outside trust mode,
-/// still inside that workspace once symlinks resolve.
-fn resolve_job_cwd(
+/// Resolve a job `cwd` against the thread workspace, never the server's cwd.
+/// Outside trust mode the directory must stay inside the workspace once
+/// symlinks resolve. This is stricter than the shell tool's `resolve_path`:
+/// the route has no `workspace_follow_symlinks` or `/trust add` roots and no
+/// `~` expansion, so a symlink leading out of the workspace is refused.
+///
+/// Containment is checked on the canonical path, but the manager receives the
+/// joined spelling, as it did before: Windows `canonicalize` returns a `\\?\`
+/// verbatim path that `cmd.exe` refuses as a current directory. Runs on
+/// `tokio::fs`, so no path resolution blocks a runtime worker.
+async fn resolve_job_cwd(
     workspace: &std::path::Path,
     raw: &str,
     trust_mode: bool,
@@ -191,14 +198,19 @@ fn resolve_job_cwd(
     } else {
         workspace.join(requested)
     };
-    let resolved = candidate
-        .canonicalize()
-        .ok()
-        .filter(|path| path.is_dir())
-        .ok_or_else(|| ApiError::bad_request("cwd must be an existing directory"))?;
+    let not_a_directory = || ApiError::bad_request("cwd must be an existing directory");
+    let resolved = tokio::fs::canonicalize(&candidate)
+        .await
+        .map_err(|_| not_a_directory())?;
+    if !tokio::fs::metadata(&resolved)
+        .await
+        .is_ok_and(|meta| meta.is_dir())
+    {
+        return Err(not_a_directory());
+    }
     if !trust_mode {
-        let root = workspace
-            .canonicalize()
+        let root = tokio::fs::canonicalize(workspace)
+            .await
             .map_err(|_| ApiError::internal("thread workspace is unavailable"))?;
         if !resolved.starts_with(&root) {
             return Err(ApiError::forbidden(
@@ -206,7 +218,11 @@ fn resolve_job_cwd(
             ));
         }
     }
-    Ok(resolved)
+    Ok(if candidate.is_absolute() {
+        candidate
+    } else {
+        resolved
+    })
 }
 
 /// `POST /v1/threads/{id}/jobs` — launch a client-owned background job under
@@ -261,12 +277,15 @@ pub(super) async fn create_thread_job(
         .map_err(|error| ApiError::forbidden(error.to_string()))?;
     // The manager resolves a relative `working_dir` against the server's own
     // cwd, so hand it the directory resolved against the thread workspace.
-    let request_cwd = request
-        .cwd
-        .as_deref()
-        .map(|cwd| resolve_job_cwd(&thread.workspace, cwd, thread.trust_mode))
-        .transpose()?
-        .map(|cwd| cwd.to_string_lossy().into_owned());
+    let request_cwd = match request.cwd.as_deref() {
+        Some(cwd) => Some(
+            resolve_job_cwd(&thread.workspace, cwd, thread.trust_mode)
+                .await?
+                .to_string_lossy()
+                .into_owned(),
+        ),
+        None => None,
+    };
     let policy = state
         .runtime_threads
         .thread_job_sandbox_policy(&thread)
@@ -595,33 +614,53 @@ pub(super) async fn resize_thread_job(
 mod tests {
     use super::*;
 
-    #[test]
-    fn job_cwd_resolves_against_the_thread_workspace() {
+    #[tokio::test]
+    async fn job_cwd_resolves_against_the_thread_workspace() {
         let tmp = tempfile::tempdir().unwrap();
         let workspace = tmp.path().join("workspace");
         std::fs::create_dir_all(workspace.join("packages/app")).unwrap();
         let outside = tmp.path().join("outside");
         std::fs::create_dir_all(&outside).unwrap();
-        let root = workspace.canonicalize().unwrap();
 
-        let resolved = resolve_job_cwd(&workspace, "packages/app", false).unwrap();
+        // Relative and absolute requests come back absolute, in the caller's
+        // spelling rather than the canonical one (no `\\?\` on Windows,
+        // no `/private` on macOS temp dirs).
+        let resolved = resolve_job_cwd(&workspace, "packages/app", false)
+            .await
+            .unwrap();
         assert!(resolved.is_absolute(), "{resolved:?}");
-        assert_eq!(resolved, root.join("packages/app"));
-        let absolute = root.join("packages").to_string_lossy().into_owned();
+        assert_eq!(resolved, workspace.join("packages/app"));
+        let absolute = workspace.join("packages");
         assert_eq!(
-            resolve_job_cwd(&workspace, &absolute, false).unwrap(),
-            root.join("packages")
+            resolve_job_cwd(&workspace, &absolute.to_string_lossy(), false)
+                .await
+                .unwrap(),
+            absolute
         );
 
-        let missing = resolve_job_cwd(&workspace, "packages/none", false).unwrap_err();
+        let missing = resolve_job_cwd(&workspace, "packages/none", false)
+            .await
+            .unwrap_err();
         assert_eq!(missing.status, StatusCode::BAD_REQUEST);
         let outside_raw = outside.to_string_lossy().into_owned();
         for raw in [outside_raw.as_str(), "../outside"] {
-            let error = resolve_job_cwd(&workspace, raw, false).unwrap_err();
+            let error = resolve_job_cwd(&workspace, raw, false).await.unwrap_err();
             assert_eq!(error.status, StatusCode::FORBIDDEN, "{raw}");
         }
+        #[cfg(unix)]
+        {
+            std::os::unix::fs::symlink(&outside, workspace.join("escape")).unwrap();
+            let error = resolve_job_cwd(&workspace, "escape", false)
+                .await
+                .unwrap_err();
+            assert_eq!(error.status, StatusCode::FORBIDDEN);
+        }
         assert_eq!(
-            resolve_job_cwd(&workspace, "../outside", true).unwrap(),
+            resolve_job_cwd(&workspace, "../outside", true)
+                .await
+                .unwrap()
+                .canonicalize()
+                .unwrap(),
             outside.canonicalize().unwrap()
         );
     }

crates/tui/src/runtime_api/workspace.rs

@@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
 
 use crate::dependencies::{ExternalTool as _, Git};
+use crate::snapshot::is_git_metadata_name;
 
 use super::{ApiError, RuntimeApiState};
 
@@ -419,33 +420,6 @@ pub(super) fn relative_request_path(raw: &str, allow_root: bool) -> Result<PathB
     Ok(path)
 }
 
-/// Whether one path component names the repository metadata directory as the
-/// filesystem resolves it, not just as spelled: `.git` in any letter case
-/// (macOS and Windows default to case-insensitive names) and, on Windows,
-/// with the trailing dots/spaces or `:stream` suffix it drops and the `GIT~N`
-/// short-name alias.
-pub(super) fn is_git_metadata_name(name: &std::ffi::OsStr) -> bool {
-    let Some(name) = name.to_str() else {
-        return false;
-    };
-    let name = if cfg!(windows) {
-        name.split(':')
-            .next()
-            .unwrap_or_default()
-            .trim_end_matches(['.', ' '])
-    } else {
-        name
-    };
-    if name.eq_ignore_ascii_case(".git") {
-        return true;
-    }
-    cfg!(windows)
-        && name.len() > 4
-        && name.is_char_boundary(4)
-        && name[..4].eq_ignore_ascii_case("git~")
-        && name[4..].bytes().all(|byte| byte.is_ascii_digit())
-}
-
 pub(super) fn canonical_workspace(workspace: &FsPath) -> Result<PathBuf, ApiError> {
     workspace
         .canonicalize()

crates/tui/src/runtime_threads/turn_artifacts.rs

@@ -156,11 +156,10 @@ fn relative_display(rel: &Path) -> Option<String> {
         let Component::Normal(name) = component else {
             return None;
         };
-        let name = name.to_str()?;
-        if name == ".git" {
+        if crate::snapshot::is_git_metadata_name(name) {
             return None;
         }
-        parts.push(name);
+        parts.push(name.to_str()?);
     }
     (!parts.is_empty()).then(|| parts.join("/"))
 }

crates/tui/src/snapshot/delta.rs

@@ -90,7 +90,9 @@ fn safe_display(path: &[u8]) -> Option<String> {
     let mut parts = Vec::new();
     for component in Path::new(text).components() {
         match component {
-            Component::Normal(name) if name != ".git" => parts.push(name.to_str()?),
+            Component::Normal(name) if !super::is_git_metadata_name(name) => {
+                parts.push(name.to_str()?)
+            }
             _ => return None,
         }
     }

crates/tui/src/snapshot/mod.rs

@@ -61,7 +61,7 @@ pub use repo::{
     DEFAULT_MAX_WORKSPACE_BYTES_FOR_SNAPSHOT, GATE_TOO_LARGE_MARKER, GATE_TOO_MANY_ENTRIES_MARKER,
     GATE_UNSAFE_LOCATION_MARKER, PathRestoreAction, PathRestoreOutcome, SIZE_WALK_MAX_ENTRIES,
     Snapshot, SnapshotId, SnapshotPathChange, SnapshotRepo, TakenSnapshot, WorkspaceGate,
-    estimate_workspace_size_bounded, workspace_relative_path,
+    estimate_workspace_size_bounded, is_git_metadata_name, workspace_relative_path,
 };
 
 /// Which point of a turn a recorded workspace snapshot captured.

crates/tui/src/snapshot/repo.rs

@@ -837,11 +837,9 @@ impl SnapshotRepo {
     /// Validate every existing component before reading, backing up or writing.
     pub fn validate_restore_file(&self, rel: &Path) -> io::Result<bool> {
         if !is_safe_relative_path(rel)
-            || rel.components().any(|part| {
-                part.as_os_str()
-                    .as_encoded_bytes()
-                    .eq_ignore_ascii_case(b".git")
-            })
+            || rel
+                .components()
+                .any(|part| is_git_metadata_name(part.as_os_str()))
         {
             return Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
@@ -2007,6 +2005,34 @@ fn is_safe_relative_path(path: &Path) -> bool {
             .all(|component| matches!(component, Component::Normal(_)))
 }
 
+/// Whether one path component names the repository metadata directory as the
+/// filesystem resolves it, not just as spelled: `.git` in any letter case
+/// (macOS and Windows default to case-insensitive names) and, on Windows,
+/// with the trailing dots/spaces or `:stream` suffix it drops and the `GIT~N`
+/// short-name alias. The one `.git` rule for workspace file routes, file
+/// restore and displayed workspace paths.
+pub fn is_git_metadata_name(name: &std::ffi::OsStr) -> bool {
+    git_metadata_name(&name.to_string_lossy(), cfg!(windows))
+}
+
+fn git_metadata_name(name: &str, windows: bool) -> bool {
+    let name = if windows {
+        name.split(':')
+            .next()
+            .unwrap_or_default()
+            .trim_end_matches(['.', ' '])
+    } else {
+        name
+    };
+    name.eq_ignore_ascii_case(".git")
+        || (windows
+            && name.len() > 4
+            && name
+                .get(..4)
+                .is_some_and(|prefix| prefix.eq_ignore_ascii_case("git~"))
+            && name[4..].bytes().all(|byte| byte.is_ascii_digit()))
+}
+
 /// Normalize a caller-supplied path into a safe workspace-relative path.
 ///
 /// Accepts either a workspace-relative path or an absolute path inside
@@ -2038,6 +2064,44 @@ mod tests {
     use std::fs::{File, FileTimes};
     use tempfile::tempdir;
 
+    #[test]
+    fn git_metadata_name_covers_case_and_windows_aliases() {
+        for windows in [false, true] {
+            for name in [".git", ".GIT", ".Git", ".gIt"] {
+                assert!(git_metadata_name(name, windows), "{name} windows={windows}");
+            }
+            for name in [
+                ".github",
+                ".gitignore",
+                "a.git",
+                "git",
+                "",
+                "git~",
+                "GIT~1a",
+            ] {
+                assert!(
+                    !git_metadata_name(name, windows),
+                    "{name} windows={windows}"
+                );
+            }
+        }
+        // Windows drops trailing dots/spaces and `:stream` suffixes, and
+        // `GIT~N` is the 8.3 short name of `.git`; elsewhere these are
+        // ordinary names.
+        for name in [
+            ".git.",
+            ".git ",
+            ".GIT. .",
+            ".git::$INDEX_ALLOCATION",
+            ".git:stream",
+            "GIT~1",
+            "git~12",
+        ] {
+            assert!(git_metadata_name(name, true), "{name}");
+            assert!(!git_metadata_name(name, false), "{name}");
+        }
+    }
+
     #[test]
     fn snapshot_id_parse_accepts_only_full_hex_object_ids() {
         let sha1 = "0123456789abcdefABCDEF0123456789abcdef01";
@@ -2541,7 +2605,11 @@ mod tests {
         let id = repo.snapshot("pre-turn:1").expect("snapshot");
         std::fs::write(dir.join("lib.rs"), b"fn b() {}").unwrap();
 
-        for rel in ["src", ".git/config", "src/.GIT/x", ".git"] {
+        let mut refused = vec!["src", ".git/config", "src/.GIT/x", ".git"];
+        if cfg!(windows) {
+            refused.extend([".git./config", ".git /config", "GIT~1/config"]);
+        }
+        for rel in refused {
             let err = repo.validate_restore_file(Path::new(rel)).expect_err(rel);
             assert_eq!(err.kind(), io::ErrorKind::InvalidInput, "{rel}");
         }

docs/RUNTIME_API.md

@@ -1585,8 +1585,10 @@ routes are the contract for now.
   thread's projected sandbox policy. `tty: true` merges stderr into stdout
   and gives the command a terminal (required for interactive programs);
   background jobs are never killed at `timeout_ms`. A relative `cwd`
-  resolves against the thread workspace, and outside trust mode `cwd` must
-  stay inside it
+  resolves against the thread workspace. Outside trust mode `cwd` must stay
+  inside it after symlinks resolve (`403` otherwise): unlike the shell tool,
+  this route does not follow `workspace_follow_symlinks` or `/trust add`
+  roots, so a symlink leading out of the workspace is refused
 - `GET /v1/threads/{id}/jobs/{job_id}` — one job's status + metadata
 - `GET /v1/threads/{id}/jobs/{job_id}/output?stream=<stdout|stderr>&cursor=
   <bytes>&max_bytes=<1-512KiB>&wait_ms=<0-30s>&format=<base64|text>` — the
@@ -1732,8 +1734,9 @@ also how a client sees model-spawned work.
   (tracked paths only — no `all`, an untracked path fails closed);
   `POST /v1/git/commit` `{ "message", "all"? }`; stage, unstage, discard
   and commit also take an optional `expect` (below); `POST /v1/git/push`
-  `{ "remote"?, "set_upstream"? }` (`remote` must name a configured
-  remote); `POST /v1/git/branch`
+  `{ "remote"?, "set_upstream"? }` (`remote`, or `origin` when only
+  `set_upstream` is given, must name a configured remote);
+  `POST /v1/git/branch`
   `{ "name", "create"? }`
 
 Diffs and precondition token reads run through the hardened review command