Skip to content

Commit 30b434a

Browse files
committed
feat: normalized config schema with PersonaRecord.provider and SPROUT_AGENT_MODEL
sprout-agent personas all failed because the desktop never injected SPROUT_AGENT_PROVIDER or model env vars at spawn time, and the goose config fallback only understood the old flat format. Extract goose config compat into an isolated submodule, add support for the newer active_provider + nested providers format, and wire a normalized config schema so Sprout injects the right env vars for any runtime. Key changes: - PersonaRecord.provider captures the LLM provider independently - SPROUT_AGENT_MODEL works as a universal model fallback - provider_env_var and model_env_var always injected at spawn time - Pack import now stores resolved env vars (was always empty) - runtime_env_vars() emits runtime-appropriate vars (SPROUT_AGENT_* vs GOOSE_*)
1 parent 9842fec commit 30b434a

19 files changed

Lines changed: 632 additions & 188 deletions

File tree

crates/sprout-acp/src/acp.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ impl AcpClient {
202202
// Callers MUST still call shutdown().await for guaranteed cleanup.
203203
.kill_on_drop(true);
204204

205-
// Per-persona env vars (e.g., GOOSE_PROVIDER, GOOSE_MODEL).
205+
// Per-persona env vars (e.g., GOOSE_PROVIDER, SPROUT_AGENT_PROVIDER).
206206
// Only injected if not already set in parent env (operator precedence).
207207
for (key, value) in extra_env {
208208
if std::env::var(key).is_err() {

crates/sprout-acp/src/config.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ pub struct Config {
466466
pub respond_to: RespondTo,
467467
/// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist).
468468
pub respond_to_allowlist: HashSet<String>,
469-
/// Per-persona env vars to inject at agent spawn time (e.g., GOOSE_PROVIDER, GOOSE_MODEL).
469+
/// Per-persona env vars to inject at agent spawn time (e.g., GOOSE_PROVIDER, GOOSE_MODEL, SPROUT_AGENT_MODEL).
470470
/// Populated from persona pack resolution. Empty when no pack is configured.
471471
pub persona_env_vars: Vec<(String, String)>,
472472
/// Whether to publish encrypted observer frames through the relay.
@@ -764,7 +764,7 @@ impl Config {
764764
(
765765
Some(persona.system_prompt),
766766
persona.model,
767-
persona.goose_env_vars,
767+
persona.runtime_env_vars,
768768
)
769769
}
770770
(Some(_), None) => {
Lines changed: 345 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,345 @@
1+
//! Goose config compatibility layer.
2+
//!
3+
//! Reads `~/.config/goose/config.yaml` to extract Databricks credentials
4+
//! as a fallback when env vars aren't set. Bridge code that shrinks as
5+
//! Sprout's spawn-time env injection improves.
6+
7+
use std::{collections::HashMap, path::PathBuf};
8+
9+
#[derive(Default)]
10+
pub(super) struct GooseDatabricksConfig {
11+
pub(super) host: Option<String>,
12+
pub(super) model: Option<String>,
13+
}
14+
15+
impl GooseDatabricksConfig {
16+
pub(super) fn load_default() -> Self {
17+
goose_config_path()
18+
.and_then(|p| Self::load_from_path(&p))
19+
.unwrap_or_default()
20+
}
21+
22+
pub(super) fn load_from_path(path: &std::path::Path) -> Option<Self> {
23+
let raw = std::fs::read_to_string(path).ok()?;
24+
let map: HashMap<String, serde_yaml::Value> = serde_yaml::from_str(&raw).ok()?;
25+
Some(Self::from_map(&map))
26+
}
27+
28+
pub(super) fn from_map(map: &HashMap<String, serde_yaml::Value>) -> Self {
29+
let host = yaml_string(map, "DATABRICKS_HOST");
30+
let explicit_model = yaml_string(map, "DATABRICKS_MODEL");
31+
let goose_provider = yaml_string(map, "GOOSE_PROVIDER");
32+
let goose_model = yaml_string(map, "GOOSE_MODEL");
33+
let goose_mode = yaml_string(map, "GOOSE_MODE");
34+
35+
// Flat-key model resolution (existing)
36+
let flat_model = explicit_model.or_else(|| {
37+
if goose_provider
38+
.as_deref()
39+
.is_some_and(|p| p.eq_ignore_ascii_case("databricks"))
40+
{
41+
goose_model.or(goose_mode)
42+
} else {
43+
None
44+
}
45+
});
46+
47+
// Nested provider format fallback (active_provider + providers block)
48+
let active_provider = yaml_string(map, "active_provider");
49+
let (nested_host, nested_model) = active_provider
50+
.as_deref()
51+
.filter(|ap| ap.to_ascii_lowercase().starts_with("databricks"))
52+
.and_then(|ap| nested_provider_config(map, ap))
53+
.unwrap_or((None, None));
54+
55+
Self {
56+
host: host.or(nested_host),
57+
model: flat_model.or(nested_model),
58+
}
59+
}
60+
}
61+
62+
fn nested_provider_config(
63+
map: &HashMap<String, serde_yaml::Value>,
64+
active_provider: &str,
65+
) -> Option<(Option<String>, Option<String>)> {
66+
let providers = map.get("providers").and_then(|v| v.as_mapping())?;
67+
let provider_config = providers
68+
.get(serde_yaml::Value::String(active_provider.to_owned()))?
69+
.as_mapping()?;
70+
71+
let model = provider_config
72+
.get(serde_yaml::Value::String("model".to_owned()))
73+
.and_then(|v| v.as_str())
74+
.map(|s| s.trim())
75+
.filter(|s| !s.is_empty())
76+
.map(str::to_string);
77+
78+
let host = provider_config
79+
.get(serde_yaml::Value::String("host".to_owned()))
80+
.and_then(|v| v.as_str())
81+
.map(|s| s.trim())
82+
.filter(|s| !s.is_empty())
83+
.map(str::to_string);
84+
85+
Some((host, model))
86+
}
87+
88+
fn yaml_string(map: &HashMap<String, serde_yaml::Value>, key: &str) -> Option<String> {
89+
map.get(key)?
90+
.as_str()
91+
.map(str::trim)
92+
.filter(|s| !s.is_empty())
93+
.map(str::to_string)
94+
}
95+
96+
fn goose_config_path() -> Option<PathBuf> {
97+
if let Ok(root) = std::env::var("GOOSE_PATH_ROOT") {
98+
return Some(PathBuf::from(root).join("config").join("config.yaml"));
99+
}
100+
let home = std::env::var("HOME").ok()?;
101+
Some(
102+
PathBuf::from(home)
103+
.join(".config")
104+
.join("goose")
105+
.join("config.yaml"),
106+
)
107+
}
108+
109+
#[cfg(test)]
110+
mod tests {
111+
use super::*;
112+
113+
// ── Existing flat-key tests ──────────────────────────────────────────────
114+
115+
#[test]
116+
fn goose_databricks_config_reads_host_and_model() {
117+
let map = HashMap::from([
118+
(
119+
"DATABRICKS_HOST".to_string(),
120+
serde_yaml::Value::String("https://dbc.example".into()),
121+
),
122+
(
123+
"GOOSE_PROVIDER".to_string(),
124+
serde_yaml::Value::String("databricks".into()),
125+
),
126+
(
127+
"GOOSE_MODEL".to_string(),
128+
serde_yaml::Value::String("goose-claude-4-6-sonnet".into()),
129+
),
130+
]);
131+
let cfg = GooseDatabricksConfig::from_map(&map);
132+
assert_eq!(cfg.host.as_deref(), Some("https://dbc.example"));
133+
assert_eq!(cfg.model.as_deref(), Some("goose-claude-4-6-sonnet"));
134+
}
135+
136+
#[test]
137+
fn goose_databricks_config_prefers_explicit_databricks_model() {
138+
let map = HashMap::from([
139+
(
140+
"DATABRICKS_HOST".to_string(),
141+
serde_yaml::Value::String("https://dbc.example".into()),
142+
),
143+
(
144+
"DATABRICKS_MODEL".to_string(),
145+
serde_yaml::Value::String("explicit-db-model".into()),
146+
),
147+
(
148+
"GOOSE_PROVIDER".to_string(),
149+
serde_yaml::Value::String("databricks".into()),
150+
),
151+
(
152+
"GOOSE_MODEL".to_string(),
153+
serde_yaml::Value::String("goose-model".into()),
154+
),
155+
]);
156+
let cfg = GooseDatabricksConfig::from_map(&map);
157+
assert_eq!(cfg.model.as_deref(), Some("explicit-db-model"));
158+
}
159+
160+
#[test]
161+
fn goose_databricks_config_ignores_goose_model_for_other_provider() {
162+
let map = HashMap::from([
163+
(
164+
"DATABRICKS_HOST".to_string(),
165+
serde_yaml::Value::String("https://dbc.example".into()),
166+
),
167+
(
168+
"GOOSE_PROVIDER".to_string(),
169+
serde_yaml::Value::String("anthropic".into()),
170+
),
171+
(
172+
"GOOSE_MODEL".to_string(),
173+
serde_yaml::Value::String("claude".into()),
174+
),
175+
]);
176+
let cfg = GooseDatabricksConfig::from_map(&map);
177+
assert_eq!(cfg.host.as_deref(), Some("https://dbc.example"));
178+
assert!(cfg.model.is_none());
179+
}
180+
181+
// ── Nested active_provider + providers block (newer goose format) ────────
182+
183+
#[test]
184+
fn from_map_reads_nested_active_provider_databricks_v2() {
185+
// Simulates:
186+
// active_provider: databricks_v2
187+
// providers:
188+
// databricks_v2:
189+
// model: goose-claude-4-6-opus
190+
// host: https://dbc.example
191+
let providers_map = {
192+
let mut inner = serde_yaml::Mapping::new();
193+
let mut provider_entry = serde_yaml::Mapping::new();
194+
provider_entry.insert(
195+
serde_yaml::Value::String("model".into()),
196+
serde_yaml::Value::String("goose-claude-4-6-opus".into()),
197+
);
198+
provider_entry.insert(
199+
serde_yaml::Value::String("host".into()),
200+
serde_yaml::Value::String("https://dbc.example".into()),
201+
);
202+
inner.insert(
203+
serde_yaml::Value::String("databricks_v2".into()),
204+
serde_yaml::Value::Mapping(provider_entry),
205+
);
206+
serde_yaml::Value::Mapping(inner)
207+
};
208+
209+
let map = HashMap::from([
210+
(
211+
"active_provider".to_string(),
212+
serde_yaml::Value::String("databricks_v2".into()),
213+
),
214+
("providers".to_string(), providers_map),
215+
]);
216+
217+
let cfg = GooseDatabricksConfig::from_map(&map);
218+
assert_eq!(cfg.host.as_deref(), Some("https://dbc.example"));
219+
assert_eq!(cfg.model.as_deref(), Some("goose-claude-4-6-opus"));
220+
}
221+
222+
#[test]
223+
fn from_map_flat_keys_win_over_nested() {
224+
// Flat DATABRICKS_MODEL takes precedence over the nested providers block.
225+
let providers_map = {
226+
let mut inner = serde_yaml::Mapping::new();
227+
let mut provider_entry = serde_yaml::Mapping::new();
228+
provider_entry.insert(
229+
serde_yaml::Value::String("model".into()),
230+
serde_yaml::Value::String("nested-model".into()),
231+
);
232+
provider_entry.insert(
233+
serde_yaml::Value::String("host".into()),
234+
serde_yaml::Value::String("https://nested-host.example".into()),
235+
);
236+
inner.insert(
237+
serde_yaml::Value::String("databricks_v2".into()),
238+
serde_yaml::Value::Mapping(provider_entry),
239+
);
240+
serde_yaml::Value::Mapping(inner)
241+
};
242+
243+
let map = HashMap::from([
244+
(
245+
"active_provider".to_string(),
246+
serde_yaml::Value::String("databricks_v2".into()),
247+
),
248+
("providers".to_string(), providers_map),
249+
(
250+
"DATABRICKS_HOST".to_string(),
251+
serde_yaml::Value::String("https://flat-host.example".into()),
252+
),
253+
(
254+
"DATABRICKS_MODEL".to_string(),
255+
serde_yaml::Value::String("flat-model".into()),
256+
),
257+
]);
258+
259+
let cfg = GooseDatabricksConfig::from_map(&map);
260+
// Flat keys win
261+
assert_eq!(cfg.host.as_deref(), Some("https://flat-host.example"));
262+
assert_eq!(cfg.model.as_deref(), Some("flat-model"));
263+
}
264+
265+
#[test]
266+
fn from_map_non_databricks_active_provider_is_ignored() {
267+
// active_provider = anthropic should not trigger nested lookup
268+
let providers_map = {
269+
let mut inner = serde_yaml::Mapping::new();
270+
let mut provider_entry = serde_yaml::Mapping::new();
271+
provider_entry.insert(
272+
serde_yaml::Value::String("model".into()),
273+
serde_yaml::Value::String("claude-opus-4".into()),
274+
);
275+
inner.insert(
276+
serde_yaml::Value::String("anthropic".into()),
277+
serde_yaml::Value::Mapping(provider_entry),
278+
);
279+
serde_yaml::Value::Mapping(inner)
280+
};
281+
282+
let map = HashMap::from([
283+
(
284+
"active_provider".to_string(),
285+
serde_yaml::Value::String("anthropic".into()),
286+
),
287+
("providers".to_string(), providers_map),
288+
]);
289+
290+
let cfg = GooseDatabricksConfig::from_map(&map);
291+
assert!(cfg.host.is_none());
292+
assert!(cfg.model.is_none());
293+
}
294+
295+
#[test]
296+
fn load_from_path_returns_none_for_nonexistent_file() {
297+
let result = GooseDatabricksConfig::load_from_path(std::path::Path::new(
298+
"/tmp/sprout-test-nonexistent-goose-config-99999999.yaml",
299+
));
300+
assert!(result.is_none());
301+
}
302+
303+
#[test]
304+
fn load_from_path_parses_valid_yaml() {
305+
let dir = tempfile::tempdir().unwrap();
306+
let path = dir.path().join("config.yaml");
307+
std::fs::write(
308+
&path,
309+
"DATABRICKS_HOST: https://dbc.example\nGOOSE_PROVIDER: databricks\nGOOSE_MODEL: goose-claude-4-6-sonnet\n",
310+
)
311+
.unwrap();
312+
let cfg = GooseDatabricksConfig::load_from_path(&path).unwrap();
313+
assert_eq!(cfg.host.as_deref(), Some("https://dbc.example"));
314+
assert_eq!(cfg.model.as_deref(), Some("goose-claude-4-6-sonnet"));
315+
}
316+
317+
#[test]
318+
fn load_from_path_returns_none_for_invalid_yaml() {
319+
let dir = tempfile::tempdir().unwrap();
320+
let path = dir.path().join("config.yaml");
321+
std::fs::write(&path, "{{{{not valid yaml at all::::").unwrap();
322+
let result = GooseDatabricksConfig::load_from_path(&path);
323+
assert!(result.is_none());
324+
}
325+
326+
#[test]
327+
fn goose_config_path_falls_back_to_home_when_root_unset() {
328+
// When GOOSE_PATH_ROOT is not set, goose_config_path() constructs a
329+
// path under $HOME. We can verify the suffix without mutating env vars.
330+
// If HOME is set (virtually all environments), the path ends with the
331+
// expected goose config suffix.
332+
if let Ok(home) = std::env::var("HOME") {
333+
// Only run the check when GOOSE_PATH_ROOT is not already set, so
334+
// this test doesn't interfere with the override logic.
335+
if std::env::var("GOOSE_PATH_ROOT").is_err() {
336+
let result = goose_config_path();
337+
let expected = std::path::PathBuf::from(&home)
338+
.join(".config")
339+
.join("goose")
340+
.join("config.yaml");
341+
assert_eq!(result, Some(expected));
342+
}
343+
}
344+
}
345+
}

0 commit comments

Comments
 (0)