From 57ff60990107c49e8cfe979ea52c0f099cc93a2e Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Mon, 14 Sep 2026 15:05:19 -0700 Subject: [PATCH] Isolate hostless runtime cache Use a dedicated runtime/ namespace for non-bundled in-process extraction while preserving direct COPILOT_CLI_EXTRACT_DIR semantics. Share path computation between build-time extraction and runtime resolution, and cover stale runtime cleanup without touching bundled CLI assets. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- rust/README.md | 22 +++++--- rust/build/in_process.rs | 41 ++------------ rust/src/cache_paths.rs | 93 +++++++++++++++++++++++++++++++ rust/src/lib.rs | 2 + rust/src/resolve.rs | 34 ++--------- rust/tests/cli_resolution_test.rs | 2 +- 6 files changed, 119 insertions(+), 75 deletions(-) create mode 100644 rust/src/cache_paths.rs diff --git a/rust/README.md b/rust/README.md index cdc8e07c05..d50dc96ec8 100644 --- a/rust/README.md +++ b/rust/README.md @@ -1073,15 +1073,19 @@ github-copilot-sdk = { version = "1", default-features = false } managed runtime artifacts directly into the platform cache using staging files and atomic renames. -3. **Runtime:** in both modes the artifacts share one versioned directory: - - | OS | Path | - |----|------| - | macOS | `~/Library/Caches/github-copilot-sdk/cli//` | - | Linux | `${XDG_CACHE_HOME:-~/.cache}/github-copilot-sdk/cli//` | - | Windows | `%LOCALAPPDATA%\github-copilot-sdk\cli\\` | - - Old version directories accumulate in siblings; clean them up at your leisure. +3. **Runtime:** embedded CLI artifacts and build-time-extracted hostless runtime + artifacts use separate versioned namespaces: + + | OS | `bundled-cli` on | `bundled-cli` off | + |----|------------------|-------------------| + | macOS | `~/Library/Caches/github-copilot-sdk/cli//` | `~/Library/Caches/github-copilot-sdk/runtime//` | + | Linux | `${XDG_CACHE_HOME:-~/.cache}/github-copilot-sdk/cli//` | `${XDG_CACHE_HOME:-~/.cache}/github-copilot-sdk/runtime//` | + | Windows | `%LOCALAPPDATA%\github-copilot-sdk\cli\\` | `%LOCALAPPDATA%\github-copilot-sdk\runtime\\` | + + Separating these namespaces prevents stale hostless-runtime cleanup during a + non-bundled build from deleting a same-version bundled CLI used by another + application. Old version directories accumulate in siblings; clean them up + at your leisure. ### Overriding the extraction location diff --git a/rust/build/in_process.rs b/rust/build/in_process.rs index c1aa583f9c..f779722e26 100644 --- a/rust/build/in_process.rs +++ b/rust/build/in_process.rs @@ -4,6 +4,9 @@ use std::time::Duration; use sha2::Digest; +#[path = "../src/cache_paths.rs"] +mod cache_paths; + pub(crate) fn main() { println!("cargo:rerun-if-env-changed=DOCS_RS"); println!("cargo:rerun-if-env-changed=COPILOT_SKIP_CLI_DOWNLOAD"); @@ -133,7 +136,7 @@ pub(crate) fn main() { // recomputes this same path from `COPILOT_SDK_CLI_VERSION` + the // OS-derived binary name + optional `COPILOT_CLI_EXTRACT_DIR`, // so we don't bake an absolute path into the crate. - let install_dir = extracted_install_dir(&version); + let install_dir = cache_paths::extracted_runtime_install_dir(&version); let required_paths = [ install_dir.join(platform.runtime_wrapper_name()), install_dir.join("runtime.node"), @@ -189,28 +192,6 @@ pub(crate) fn main() { } } -/// Install directory used when `bundled-cli` is off. Mirrors the runtime -/// convention in `src/resolve.rs::extracted_cli_path`: both sides MUST -/// compute the same path from the same inputs, otherwise the runtime -/// resolver won't find what build.rs extracted. -/// -/// If `COPILOT_CLI_EXTRACT_DIR` is set the binary lives directly under -/// that directory (no per-version subdir) — useful for vendored slots and -/// for `.cargo/config.toml [env]`-style pinning that's symmetric between -/// build-time write and runtime read. Otherwise the binary lives under -/// `/github-copilot-sdk/cli//`. -fn extracted_install_dir(version: &str) -> PathBuf { - if let Some(custom) = std::env::var_os("COPILOT_CLI_EXTRACT_DIR") { - PathBuf::from(custom) - } else { - let cache = dirs::cache_dir().unwrap_or_else(std::env::temp_dir); - cache - .join("github-copilot-sdk") - .join("cli") - .join(sanitize_version(version)) - } -} - /// Emit separate full-CLI and runtime payloads into `OUT_DIR` for embed mode. fn emit_embedded( out: &Path, @@ -785,20 +766,6 @@ fn install_cached_file_path( } } -/// Replace characters outside `[a-zA-Z0-9._-]` with `_` so the version -/// string is always safe to use as a path component. Kept in sync with -/// `embeddedcli::sanitize_version` and `resolve::sanitize_version` so all -/// three resolve to the same cache directory for any given version. -fn sanitize_version(version: &str) -> String { - version - .chars() - .map(|c| match c { - 'a'..='z' | 'A'..='Z' | '0'..='9' | '.' | '-' | '_' => c, - _ => '_', - }) - .collect() -} - /// Read a file from the download cache, or download it (with retries) and save /// to cache. Verifies SHA-256 on every path. Evicts stale/corrupt cache entries /// automatically. Cache I/O failures are treated as cache misses — they never diff --git a/rust/src/cache_paths.rs b/rust/src/cache_paths.rs new file mode 100644 index 0000000000..116e235be1 --- /dev/null +++ b/rust/src/cache_paths.rs @@ -0,0 +1,93 @@ +use std::ffi::OsStr; +use std::path::{Path, PathBuf}; + +const SDK_CACHE_DIR: &str = "github-copilot-sdk"; +#[cfg(test)] +const CLI_CACHE_DIR: &str = "cli"; +const RUNTIME_CACHE_DIR: &str = "runtime"; + +pub(crate) fn extracted_runtime_install_dir(version: &str) -> PathBuf { + runtime_install_dir( + std::env::var_os("COPILOT_CLI_EXTRACT_DIR").as_deref(), + &platform_cache_dir(), + version, + ) +} + +fn runtime_install_dir(custom_dir: Option<&OsStr>, cache_root: &Path, version: &str) -> PathBuf { + match custom_dir { + Some(custom_dir) => PathBuf::from(custom_dir), + None => cache_install_dir(cache_root, RUNTIME_CACHE_DIR, version), + } +} + +fn cache_install_dir(cache_root: &Path, namespace: &str, version: &str) -> PathBuf { + cache_root + .join(SDK_CACHE_DIR) + .join(namespace) + .join(version_component(version)) +} + +fn platform_cache_dir() -> PathBuf { + dirs::cache_dir().unwrap_or_else(std::env::temp_dir) +} + +fn version_component(version: &str) -> String { + version + .chars() + .map(|c| match c { + 'a'..='z' | 'A'..='Z' | '0'..='9' | '.' | '-' | '_' => c, + _ => '_', + }) + .collect() +} + +#[cfg(test)] +mod tests { + use std::ffi::OsStr; + use std::fs; + use std::path::{Path, PathBuf}; + + use tempfile::tempdir; + + use super::{CLI_CACHE_DIR, cache_install_dir, runtime_install_dir}; + + #[test] + fn custom_runtime_directory_is_used_directly() { + let cache = Path::new("ignored-cache"); + let custom = OsStr::new("custom-runtime"); + + assert_eq!( + runtime_install_dir(Some(custom), cache, "1.2.3"), + PathBuf::from(custom) + ); + } + + #[test] + fn stale_runtime_cleanup_cannot_remove_same_version_bundled_cli() { + let cache = tempdir().expect("create cache root"); + let version = "1.2.3/test"; + let bundled_cli_dir = cache_install_dir(cache.path(), CLI_CACHE_DIR, version); + let runtime_dir = runtime_install_dir(None, cache.path(), version); + let bundled_cli = bundled_cli_dir.join(if cfg!(windows) { + "copilot.exe" + } else { + "copilot" + }); + let runtime_marker = runtime_dir.join(".hostless-runtime-assets-v1"); + + fs::create_dir_all(&bundled_cli_dir).expect("create bundled CLI directory"); + fs::write(&bundled_cli, b"bundled-cli").expect("write bundled CLI"); + fs::create_dir_all(&runtime_dir).expect("create runtime directory"); + fs::write(&runtime_marker, b"stale").expect("write stale runtime marker"); + + assert_ne!(runtime_dir, bundled_cli_dir); + fs::remove_dir_all(&runtime_dir).expect("clear stale runtime directory"); + + assert_eq!( + fs::read(&bundled_cli).expect("bundled CLI survives runtime cleanup"), + b"bundled-cli" + ); + assert!(!runtime_marker.exists()); + } +} diff --git a/rust/src/lib.rs b/rust/src/lib.rs index c95ed2087a..7e0be532d5 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -3,6 +3,8 @@ #![deny(rustdoc::broken_intra_doc_links)] #![cfg_attr(test, allow(clippy::unwrap_used))] +#[cfg(not(feature = "bundled-cli"))] +mod cache_paths; /// Canvas declarations, provider callbacks, and host-side canvas RPC types. pub mod canvas; mod canvas_dispatch; diff --git a/rust/src/resolve.rs b/rust/src/resolve.rs index d8b996a11a..c72610b3ec 100644 --- a/rust/src/resolve.rs +++ b/rust/src/resolve.rs @@ -103,22 +103,15 @@ pub(crate) fn copilot_binary_with_extract_dir( /// The path is recomputed from the build-time-baked /// `COPILOT_SDK_CLI_VERSION`, the OS-derived binary name, and the /// optional `COPILOT_CLI_EXTRACT_DIR` env var. This must match -/// `build.rs::extracted_install_dir` exactly — both sides implement the -/// same convention. We deliberately don't bake the resolved path into -/// the crate at build time: an absolute path leaks the build machine's -/// `$HOME` / `$LOCALAPPDATA` into the artifact, breaks sccache across -/// machines, and prevents copying `target/` between hosts. +/// the build script exactly; both use `cache_paths` so the convention +/// cannot drift. We deliberately don't bake the resolved path into the +/// crate at build time: an absolute path leaks the build machine's `$HOME` +/// / `$LOCALAPPDATA` into the artifact, breaks sccache across machines, +/// and prevents copying `target/` between hosts. #[cfg(all(not(feature = "bundled-cli"), has_extracted_cli))] fn extracted_program(use_runtime_wrapper: bool) -> Option { let version = env!("COPILOT_SDK_CLI_VERSION"); - let dir = match env::var_os("COPILOT_CLI_EXTRACT_DIR") { - Some(custom) => PathBuf::from(custom), - None => dirs::cache_dir() - .unwrap_or_else(env::temp_dir) - .join("github-copilot-sdk") - .join("cli") - .join(sanitize_version(version)), - }; + let dir = crate::cache_paths::extracted_runtime_install_dir(version); let path = dir.join(if use_runtime_wrapper { runtime_binary_name() @@ -220,21 +213,6 @@ fn runtime_binary_name() -> &'static str { } } -/// Replace characters outside `[a-zA-Z0-9._-]` with `_`. Kept in sync -/// with `build.rs::sanitize_version` and `embeddedcli::sanitize_version` -/// so all three resolve to the same cache directory for any given -/// version. -#[cfg(all(not(feature = "bundled-cli"), has_extracted_cli))] -fn sanitize_version(version: &str) -> String { - version - .chars() - .map(|c| match c { - 'a'..='z' | 'A'..='Z' | '0'..='9' | '.' | '-' | '_' => c, - _ => '_', - }) - .collect() -} - #[cfg(test)] mod tests { use std::fs; diff --git a/rust/tests/cli_resolution_test.rs b/rust/tests/cli_resolution_test.rs index 846fbacd27..c8adf5fbe5 100644 --- a/rust/tests/cli_resolution_test.rs +++ b/rust/tests/cli_resolution_test.rs @@ -116,7 +116,7 @@ fn extracted_binary_present_at_conventional_path() { let path = dirs::cache_dir() .expect("platform cache dir") .join("github-copilot-sdk") - .join("cli") + .join("runtime") .join(sanitized) .join(binary); assert!(