From e3815af69b32ba7c741935297067bbe2721c864e Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Fri, 19 Jun 2026 11:44:16 -0600 Subject: [PATCH] fix: sbx mixins must be passed in directories, not as files and the files must be named spec.yaml per new sbx version --- src/config/mod.rs | 1 + src/config/paths.rs | 8 +- src/sandbox/mixins.rs | 205 ++++++++++++++++++++++++++++++++++++++++++ src/sandbox/mod.rs | 29 ++++-- 4 files changed, 233 insertions(+), 10 deletions(-) diff --git a/src/config/mod.rs b/src/config/mod.rs index c503094..2150683 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -147,6 +147,7 @@ const SBX_KIT_DIR_NAME: &str = "sbx-kit"; const SBX_KIT_HASH_FILE: &str = "kit.sha256"; const SBX_MIXIN_FILE_NAME: &str = "sbx-mixin.yaml"; const SBX_VAULT_MIXINS_DIR_NAME: &str = "sbx-vault-mixins"; +const SBX_MIXIN_KITS_DIR_NAME: &str = "sbx-mixin-kits"; const GIT_DIR_NAME: &str = ".git"; const GITIGNORE_FILE_NAME: &str = ".gitignore"; const DEFAULT_VISIBLE_TOOLS: [&str; 18] = [ diff --git a/src/config/paths.rs b/src/config/paths.rs index a409492..3d08d5e 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -4,8 +4,8 @@ use super::{ ENV_FILE_NAME, FUNCTIONS_BIN_DIR_NAME, FUNCTIONS_DIR_NAME, GLOBAL_TOOLS_DIR_NAME, GLOBAL_TOOLS_UTILS_DIR_NAME, MACROS_DIR_NAME, MCP_FILE_NAME, MEMORY_DIR_NAME, MEMORY_INDEX_FILE_NAME, ModelsOverride, RAGS_DIR_NAME, ROLES_DIR_NAME, SBX_KIT_DIR_NAME, - SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME, SBX_VAULT_MIXINS_DIR_NAME, SKILLS_DIR_NAME, - WORKSPACE_MEMORY_DIR_NAME, + SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME, SBX_MIXIN_KITS_DIR_NAME, SBX_VAULT_MIXINS_DIR_NAME, + SKILLS_DIR_NAME, WORKSPACE_MEMORY_DIR_NAME, }; use crate::client::ProviderModels; use crate::utils::{get_env_name, list_file_names, normalize_env_name}; @@ -154,6 +154,10 @@ pub fn sbx_vault_mixins_hash_file() -> PathBuf { sbx_vault_mixins_dir().join(SBX_KIT_HASH_FILE) } +pub fn sbx_mixin_kits_dir() -> PathBuf { + cache_path().join(SBX_MIXIN_KITS_DIR_NAME) +} + pub fn config_file() -> PathBuf { match env::var(get_env_name("config_file")) { Ok(value) => PathBuf::from(value), diff --git a/src/sandbox/mixins.rs b/src/sandbox/mixins.rs index 0fe9e0d..9a5d8f2 100644 --- a/src/sandbox/mixins.rs +++ b/src/sandbox/mixins.rs @@ -1,13 +1,16 @@ use std::env; +use std::fs; use std::fs::{read_dir, read_to_string}; use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use serde_yaml::Value; +use sha2::{Digest, Sha256}; use crate::config::paths; const SBX_MIXIN_FILE_NAME: &str = "sbx-mixin.yaml"; +const KIT_SPEC_FILE_NAME: &str = "spec.yaml"; #[derive(Debug, Clone)] pub struct DiscoveredMixin { @@ -17,6 +20,46 @@ pub struct DiscoveredMixin { pub domain_count: usize, } +impl DiscoveredMixin { + pub fn kit_path(&self) -> Result { + if self.path.is_dir() { + return Ok(self.path.clone()); + } + + wrap_mixin_as_kit(&self.path) + } +} + +pub fn wrap_mixin_as_kit(mixin_path: &Path) -> Result { + let bytes = fs::read(mixin_path) + .with_context(|| format!("Failed to read sbx mixin {}", mixin_path.display()))?; + let mut hasher = Sha256::new(); + hasher.update(&bytes); + let hash = format!("{:x}", hasher.finalize()); + + let kit_dir = paths::sbx_mixin_kits_dir().join(&hash); + let spec_path = kit_dir.join(KIT_SPEC_FILE_NAME); + + if let Ok(existing) = fs::read(&spec_path) + && existing == bytes + { + return Ok(kit_dir); + } + + fs::create_dir_all(&kit_dir) + .with_context(|| format!("Failed to create mixin kit dir {}", kit_dir.display()))?; + fs::write(&spec_path, &bytes) + .with_context(|| format!("Failed to write {}", spec_path.display()))?; + + debug!( + "Wrapped mixin {} as kit at {}", + mixin_path.display(), + kit_dir.display() + ); + + Ok(kit_dir) +} + pub fn discover() -> Result> { let mut out = Vec::new(); @@ -234,4 +277,166 @@ network: let found = collect_subdir_mixins(&absent); assert!(found.is_empty()); } + + mod wrap_as_kit { + use super::*; + use serial_test::serial; + use std::ffi::OsString; + + struct TestCacheDirGuard { + key: String, + previous: Option, + path: PathBuf, + } + + impl TestCacheDirGuard { + fn new() -> Self { + let key = crate::utils::get_env_name("cache_dir"); + let previous = env::var_os(&key); + let nanos = time::SystemTime::now() + .duration_since(time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let path = env::temp_dir().join(format!("coyote-mixin-wrap-cache-{nanos}")); + fs::create_dir_all(&path).unwrap(); + unsafe { + env::set_var(&key, &path); + } + Self { + key, + previous, + path, + } + } + } + + impl Drop for TestCacheDirGuard { + fn drop(&mut self) { + unsafe { + match &self.previous { + Some(v) => env::set_var(&self.key, v), + None => env::remove_var(&self.key), + } + } + let _ = fs::remove_dir_all(&self.path); + } + } + + fn write_mixin(name: &str, content: &str) -> PathBuf { + let root = unique_root(&format!("wrap-src-{name}")); + let path = root.join("sbx-mixin.yaml"); + fs::write(&path, content).unwrap(); + path + } + + #[test] + #[serial] + fn wrap_mixin_as_kit_creates_spec_yaml_with_original_content() { + let _guard = TestCacheDirGuard::new(); + let content = "schemaVersion: \"1\"\nkind: mixin\nname: probe\n"; + let mixin = write_mixin("content", content); + + let kit_dir = wrap_mixin_as_kit(&mixin).unwrap(); + let spec = kit_dir.join("spec.yaml"); + + assert!(spec.exists(), "spec.yaml must exist in wrapped kit dir"); + assert_eq!(fs::read_to_string(&spec).unwrap(), content); + } + + #[test] + #[serial] + fn wrap_mixin_as_kit_is_deterministic_for_identical_content() { + let _guard = TestCacheDirGuard::new(); + let content = "schemaVersion: \"1\"\nkind: mixin\nname: probe\n"; + let mixin_one = write_mixin("dedup-1", content); + let mixin_two = write_mixin("dedup-2", content); + + let kit_a = wrap_mixin_as_kit(&mixin_one).unwrap(); + let kit_b = wrap_mixin_as_kit(&mixin_two).unwrap(); + + assert_eq!( + kit_a, kit_b, + "same content should share the same content-addressed kit dir" + ); + } + + #[test] + #[serial] + fn wrap_mixin_as_kit_different_content_yields_different_dirs() { + let _guard = TestCacheDirGuard::new(); + let mixin_a = write_mixin("diff-a", "kind: mixin\nname: a\n"); + let mixin_b = write_mixin("diff-b", "kind: mixin\nname: b\n"); + + let kit_a = wrap_mixin_as_kit(&mixin_a).unwrap(); + let kit_b = wrap_mixin_as_kit(&mixin_b).unwrap(); + + assert_ne!( + kit_a, kit_b, + "different content must hash to different kit dirs" + ); + } + + #[test] + #[serial] + fn wrap_mixin_as_kit_is_idempotent_on_cache_hit() { + let _guard = TestCacheDirGuard::new(); + let mixin = write_mixin("idempotent", "kind: mixin\nname: probe\n"); + + let kit_first = wrap_mixin_as_kit(&mixin).unwrap(); + let spec = kit_first.join("spec.yaml"); + let mtime_first = fs::metadata(&spec).unwrap().modified().unwrap(); + + std::thread::sleep(std::time::Duration::from_millis(10)); + + let kit_second = wrap_mixin_as_kit(&mixin).unwrap(); + let mtime_second = fs::metadata(kit_second.join("spec.yaml")) + .unwrap() + .modified() + .unwrap(); + + assert_eq!(kit_first, kit_second); + assert_eq!( + mtime_first, mtime_second, + "cache hit must not rewrite spec.yaml" + ); + } + + #[test] + #[serial] + fn kit_path_passes_through_existing_directory() { + let _guard = TestCacheDirGuard::new(); + let dir = unique_root("kit-path-dir-passthrough"); + + let m = DiscoveredMixin { + path: dir.clone(), + label: "vault".into(), + install_count: 1, + domain_count: 1, + }; + + assert_eq!(m.kit_path().unwrap(), dir); + } + + #[test] + #[serial] + fn kit_path_wraps_file_into_kit_dir() { + let _guard = TestCacheDirGuard::new(); + let mixin = write_mixin("kit-path-wrap", "kind: mixin\nname: probe\n"); + + let m = DiscoveredMixin { + path: mixin.clone(), + label: mixin.display().to_string(), + install_count: 0, + domain_count: 0, + }; + + let wrapped = m.kit_path().unwrap(); + assert!(wrapped.is_dir(), "kit_path of a file should be a directory"); + assert!(wrapped.join("spec.yaml").exists()); + assert_ne!( + wrapped, mixin, + "kit_path should not return the original file path" + ); + } + } } diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 244899f..12582f9 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -347,12 +347,13 @@ fn build_create_args( ]; for mixin in mixins { - let mixin_str = mixin - .path + let mixin_kit = mixin.kit_path()?; + let mixin_str = mixin_kit .to_str() - .ok_or_else(|| anyhow!("Mixin path is not valid UTF-8: {}", mixin.path.display()))?; + .ok_or_else(|| anyhow!("Mixin kit path is not valid UTF-8: {}", mixin_kit.display()))? + .to_string(); args.push("--kit".to_string()); - args.push(mixin_str.to_string()); + args.push(mixin_str); } args.push(SANDBOX_AGENT.to_string()); @@ -590,15 +591,24 @@ mod tests { #[test] fn build_create_args_emits_base_kit_before_mixins() { let kit = PathBuf::from("/cache/sbx-kit"); + let unique = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let dir_a = env::temp_dir().join(format!("coyote-mixin-a-{unique}")); + let dir_b = env::temp_dir().join(format!("coyote-mixin-b-{unique}")); + fs::create_dir_all(&dir_a).unwrap(); + fs::create_dir_all(&dir_b).unwrap(); + let mixins = vec![ DiscoveredMixin { - path: PathBuf::from("/cfg/sbx-mixin.yaml"), + path: dir_a.clone(), label: "user".into(), install_count: 0, domain_count: 0, }, DiscoveredMixin { - path: PathBuf::from("/cfg/agents/sql/sbx-mixin.yaml"), + path: dir_b.clone(), label: "sql".into(), install_count: 0, domain_count: 0, @@ -614,15 +624,18 @@ mod tests { "--kit".to_string(), "/cache/sbx-kit".to_string(), "--kit".to_string(), - "/cfg/sbx-mixin.yaml".to_string(), + dir_a.display().to_string(), "--kit".to_string(), - "/cfg/agents/sql/sbx-mixin.yaml".to_string(), + dir_b.display().to_string(), "coyote".to_string(), "--name".to_string(), "my-box".to_string(), ".".to_string(), ] ); + + let _ = fs::remove_dir_all(&dir_a); + let _ = fs::remove_dir_all(&dir_b); } #[test]