Skip to content

Commit ea1a9a6

Browse files
npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6wtellaho
andcommitted
refactor(desktop): remove orphaned model-discovery probe
The agent profile sidebar went read-only, removing the ModelPicker that was the sole consumer of the model-discovery probe (`get_agent_models`, which spawns `buzz-acp models --json`). Rip out the now-dead chain: - delete ModelPicker.tsx (imported by nothing) - drop the get_agent_models Tauri command + its probe/normalization helpers in agent_models.rs, its lib.rs registration, and the e2eBridge mock - remove AgentModelsResponse/AgentModelInfo (Rust + TS) and the getAgentModels() wrapper - remove the three helpers the probe was the only caller of: resolve_persona_env, redact_env_values_in (a thin redact_secrets_with wrapper), and resolve_effective_prompt_model_provider, plus their now-orphaned tests Keep update_managed_agent (lives in the same file, still used widely) and the static "Model: Auto" display row (pure config read). Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
1 parent 9a26425 commit ea1a9a6

11 files changed

Lines changed: 6 additions & 709 deletions

File tree

Lines changed: 4 additions & 321 deletions
Original file line numberDiff line numberDiff line change
@@ -1,205 +1,18 @@
1-
use std::collections::HashSet;
2-
31
use nostr::Keys;
42
use tauri::{AppHandle, State};
53

64
use crate::{
75
app_state::AppState,
86
managed_agents::{
9-
build_databricks_defaults, build_managed_agent_summary, current_instance_id,
10-
default_agent_workdir, find_managed_agent_mut, known_acp_runtime, load_managed_agents,
11-
load_personas, managed_agent_avatar_url, missing_command_message, normalize_agent_args,
12-
resolve_command, resolve_effective_prompt_model_provider, runtime_metadata_env_vars,
13-
save_managed_agents, sync_managed_agent_processes, try_regenerate_nest, AgentModelInfo,
14-
AgentModelsResponse, UpdateManagedAgentRequest, UpdateManagedAgentResponse,
7+
build_managed_agent_summary, current_instance_id, find_managed_agent_mut,
8+
load_managed_agents, load_personas, managed_agent_avatar_url, save_managed_agents,
9+
sync_managed_agent_processes, try_regenerate_nest, UpdateManagedAgentRequest,
10+
UpdateManagedAgentResponse,
1511
},
1612
relay::{relay_ws_url_with_override, sync_managed_agent_profile},
1713
util::now_iso,
1814
};
1915

20-
/// Query available models from an agent via `buzz-acp models --json`.
21-
///
22-
/// Spawns a short-lived subprocess (no relay connection needed). The subprocess
23-
/// starts the agent, queries its model catalog, and exits. ~2-5s total.
24-
#[tauri::command]
25-
pub async fn get_agent_models(
26-
pubkey: String,
27-
app: AppHandle,
28-
state: State<'_, AppState>,
29-
) -> Result<AgentModelsResponse, String> {
30-
let (
31-
resolved_acp,
32-
agent_command,
33-
agent_args,
34-
persisted_model,
35-
runtime_default_env,
36-
runtime_metadata_env,
37-
databricks_defaults,
38-
merged_env,
39-
) = {
40-
let _store_guard = state
41-
.managed_agents_store_lock
42-
.lock()
43-
.map_err(|e| e.to_string())?;
44-
let mut records = load_managed_agents(&app)?;
45-
let mut runtimes = state
46-
.managed_agent_processes
47-
.lock()
48-
.map_err(|e| e.to_string())?;
49-
if sync_managed_agent_processes(&mut records, &mut runtimes, &current_instance_id(&app)) {
50-
save_managed_agents(&app, &records)?;
51-
}
52-
53-
let record = records
54-
.iter()
55-
.find(|r| r.pubkey == pubkey)
56-
.ok_or_else(|| format!("agent {pubkey} not found"))?;
57-
58-
let resolved = resolve_command(&record.acp_command)
59-
.ok_or_else(|| missing_command_message(&record.acp_command, "ACP harness command"))?;
60-
61-
// Resolve the effective harness from the linked persona (mirrors spawn),
62-
// so model discovery runs against the persona's current harness, not the
63-
// frozen record snapshot. An explicit per-agent override wins.
64-
let personas = load_personas(&app).unwrap_or_default();
65-
let effective_command = crate::managed_agents::effective_agent_command(
66-
record.persona_id.as_deref(),
67-
&personas,
68-
record.agent_command_override.as_deref(),
69-
);
70-
71-
let args = normalize_agent_args(&effective_command, record.agent_args.clone());
72-
73-
let resolved_agent = resolve_command(&effective_command)
74-
.map(|p| p.display().to_string())
75-
.unwrap_or_else(|| effective_command.clone());
76-
77-
// Same env layering as runtime spawn: persona env < agent env.
78-
// Model discovery needs the user's credentials. Fail closed on
79-
// persona-resolution errors so a corrupt personas.json doesn't
80-
// produce a model list as if the persona had no credentials.
81-
let persona_env =
82-
crate::managed_agents::resolve_persona_env(&app, record.persona_id.as_deref())?;
83-
let env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars);
84-
85-
// Resolve the effective model from the linked persona so the ModelPicker
86-
// dropdown shows the current persona model as selected.
87-
let (_prompt, effective_model, effective_provider) =
88-
resolve_effective_prompt_model_provider(
89-
record.persona_id.as_deref(),
90-
&personas,
91-
record.system_prompt.clone(),
92-
record.model.clone(),
93-
);
94-
let runtime = known_acp_runtime(&record.agent_command);
95-
let runtime_default_env: Vec<(String, String)> = runtime
96-
.map(|meta| {
97-
meta.default_env
98-
.iter()
99-
.map(|(key, value)| ((*key).to_string(), (*value).to_string()))
100-
.collect()
101-
})
102-
.unwrap_or_default();
103-
let runtime_metadata_env: Vec<(String, String)> = runtime
104-
.map(|meta| {
105-
runtime_metadata_env_vars(
106-
meta.model_env_var,
107-
meta.provider_env_var,
108-
meta.provider_locked,
109-
effective_model.as_deref(),
110-
effective_provider.as_deref(),
111-
)
112-
.into_iter()
113-
.map(|(key, value)| (key.to_string(), value.to_string()))
114-
.collect()
115-
})
116-
.unwrap_or_default();
117-
let databricks_defaults: Vec<(String, String)> = build_databricks_defaults()
118-
.into_iter()
119-
.map(|(key, value)| (key.to_string(), value.to_string()))
120-
.collect();
121-
122-
(
123-
resolved,
124-
resolved_agent,
125-
args,
126-
effective_model,
127-
runtime_default_env,
128-
runtime_metadata_env,
129-
databricks_defaults,
130-
env,
131-
)
132-
}; // store lock released — subprocess runs without holding the lock
133-
134-
// Clone the env map for redaction below — `merged_env` is moved
135-
// into the spawn_blocking closure and we still need the values to
136-
// scrub any user-supplied secrets that the child surfaces in stderr.
137-
let env_for_redaction = merged_env.clone();
138-
139-
// Use spawn_blocking because the desktop Tauri crate doesn't enable
140-
// tokio's `process` feature. std::process::Command is synchronous
141-
// but fine for a short-lived subprocess (~2-5s).
142-
let output = tokio::task::spawn_blocking(move || {
143-
let mut cmd = std::process::Command::new(&resolved_acp);
144-
if let Some(home) = default_agent_workdir() {
145-
cmd.current_dir(home);
146-
}
147-
if let Some(ref path) = crate::managed_agents::login_shell_path() {
148-
cmd.env("PATH", path);
149-
}
150-
cmd.arg("models")
151-
.arg("--json")
152-
.env("BUZZ_ACP_AGENT_COMMAND", &agent_command)
153-
.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(","));
154-
for (key, value) in &runtime_default_env {
155-
if std::env::var(key).is_err() {
156-
cmd.env(key, value);
157-
}
158-
}
159-
for (key, value) in &runtime_metadata_env {
160-
cmd.env(key, value);
161-
}
162-
for (key, value) in &databricks_defaults {
163-
cmd.env(key, value);
164-
}
165-
// User env layering — written LAST so it overrides any Buzz-set env above.
166-
for (k, v) in &merged_env {
167-
cmd.env(k, v);
168-
}
169-
cmd.stdout(std::process::Stdio::piped())
170-
.stderr(std::process::Stdio::piped())
171-
.output()
172-
.map_err(|e| format!("failed to spawn buzz-acp models: {e}"))
173-
})
174-
.await
175-
.map_err(|e| format!("model discovery task failed: {e}"))?
176-
.map_err(|e: String| e)?;
177-
178-
if !output.status.success() {
179-
let stderr = String::from_utf8_lossy(&output.stderr);
180-
// Scrub any user-supplied env values before surfacing stderr to
181-
// the frontend — persona/agent env_vars may carry API keys that
182-
// a failing child process echoed back.
183-
let stderr_redacted =
184-
crate::managed_agents::redact_env_values_in(stderr.as_ref(), &env_for_redaction);
185-
if let Some(configuration_error) = model_configuration_error(&stderr_redacted) {
186-
return Ok(unavailable_agent_models(
187-
persisted_model,
188-
configuration_error,
189-
));
190-
}
191-
return Err(format!(
192-
"buzz-acp models failed (exit {}): {stderr_redacted}",
193-
output.status.code().unwrap_or(-1)
194-
));
195-
}
196-
197-
let raw: serde_json::Value = serde_json::from_slice(&output.stdout)
198-
.map_err(|e| format!("failed to parse model JSON: {e}"))?;
199-
200-
Ok(normalize_agent_models(&raw, persisted_model))
201-
}
202-
20316
/// Update mutable fields on an existing managed agent record.
20417
///
20518
/// Does NOT auto-restart the agent. Runtime config changes (system prompt,
@@ -389,133 +202,3 @@ pub async fn update_managed_agent(
389202
profile_sync_error,
390203
})
391204
}
392-
393-
// ── Model normalization ───────────────────────────────────────────────────────
394-
395-
/// Normalize raw `buzz-acp models --json` output into a typed DTO for the frontend.
396-
///
397-
/// Merges models from both ACP paths (stable configOptions + unstable SessionModelState),
398-
/// deduplicates by ID (stable takes precedence), and returns a unified list.
399-
fn normalize_agent_models(
400-
raw: &serde_json::Value,
401-
persisted_model: Option<String>,
402-
) -> AgentModelsResponse {
403-
let agent_name = raw["agent"]["name"]
404-
.as_str()
405-
.unwrap_or("unknown")
406-
.to_string();
407-
let agent_version = raw["agent"]["version"]
408-
.as_str()
409-
.unwrap_or("unknown")
410-
.to_string();
411-
412-
let mut models: Vec<AgentModelInfo> = Vec::new();
413-
let mut seen_ids: HashSet<String> = HashSet::new();
414-
415-
// 1. Stable configOptions (preferred). Only entries with category "model"
416-
// are model options — the CLI pre-filters, but we're defensive here.
417-
if let Some(config_options) = raw["stable"]["configOptions"].as_array() {
418-
for opt in config_options {
419-
if opt.get("category").and_then(|c| c.as_str()) != Some("model") {
420-
continue;
421-
}
422-
if let Some(options) = opt.get("options").and_then(|v| v.as_array()) {
423-
for o in options {
424-
if let Some(value) = o.get("value").and_then(|v| v.as_str()) {
425-
if seen_ids.insert(value.to_string()) {
426-
models.push(AgentModelInfo {
427-
id: value.to_string(),
428-
name: o
429-
.get("displayName")
430-
.and_then(|v| v.as_str())
431-
.map(str::to_string),
432-
description: None,
433-
});
434-
}
435-
}
436-
}
437-
}
438-
}
439-
}
440-
441-
// 2. Unstable availableModels (fallback — skip duplicates from stable).
442-
let mut agent_default_model: Option<String> = None;
443-
if let Some(unstable) = raw.get("unstable") {
444-
agent_default_model = unstable["currentModelId"].as_str().map(str::to_string);
445-
if let Some(available) = unstable["availableModels"].as_array() {
446-
for m in available {
447-
if let Some(id) = m.get("modelId").and_then(|v| v.as_str()) {
448-
if seen_ids.insert(id.to_string()) {
449-
models.push(AgentModelInfo {
450-
id: id.to_string(),
451-
name: m.get("name").and_then(|v| v.as_str()).map(str::to_string),
452-
description: m
453-
.get("description")
454-
.and_then(|v| v.as_str())
455-
.map(str::to_string),
456-
});
457-
}
458-
}
459-
}
460-
}
461-
}
462-
463-
let supports_switching = !models.is_empty();
464-
465-
AgentModelsResponse {
466-
agent_name,
467-
agent_version,
468-
models,
469-
agent_default_model,
470-
selected_model: persisted_model,
471-
supports_switching,
472-
configuration_error: None,
473-
}
474-
}
475-
476-
fn unavailable_agent_models(
477-
persisted_model: Option<String>,
478-
configuration_error: String,
479-
) -> AgentModelsResponse {
480-
AgentModelsResponse {
481-
agent_name: "unknown".to_string(),
482-
agent_version: "unknown".to_string(),
483-
models: Vec::new(),
484-
agent_default_model: None,
485-
selected_model: persisted_model,
486-
supports_switching: false,
487-
configuration_error: Some(configuration_error),
488-
}
489-
}
490-
491-
fn model_configuration_error(stderr: &str) -> Option<String> {
492-
let normalized = stderr.to_ascii_lowercase();
493-
494-
if normalized.contains("buzz_agent_provider required") {
495-
return Some(
496-
"This agent does not have an LLM provider configured. Set a provider and model on the persona or agent, then retry."
497-
.to_string(),
498-
);
499-
}
500-
501-
if normalized.contains("anthropic_model required")
502-
|| normalized.contains("openai_compat_model required")
503-
|| normalized.contains("databricks_model required")
504-
{
505-
return Some(
506-
"This agent does not have an LLM model configured. Set a model on the persona or agent, then retry."
507-
.to_string(),
508-
);
509-
}
510-
511-
if normalized.contains("anthropic_api_key required")
512-
|| normalized.contains("openai_compat_api_key required")
513-
{
514-
return Some(
515-
"This agent is missing credentials for its configured LLM provider. Add the provider credentials, then retry."
516-
.to_string(),
517-
);
518-
}
519-
520-
None
521-
}

desktop/src-tauri/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -825,7 +825,6 @@ pub fn run() {
825825
set_managed_agent_start_on_app_launch,
826826
delete_managed_agent,
827827
get_managed_agent_log,
828-
get_agent_models,
829828
mesh_availability,
830829
mesh_start_node,
831830
mesh_ensure_client_node,

desktop/src-tauri/src/managed_agents/backend.rs

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -337,22 +337,6 @@ fn env_secrets_from_request(request: &serde_json::Value) -> Vec<String> {
337337
.unwrap_or_default()
338338
}
339339

340-
/// Public-in-crate helper: redact every non-empty value from `env` (plus
341-
/// the standard nsec/sprt_tok prefix scrubbing) out of `s`. Used by
342-
/// callers that already have a flat env map handy — e.g. model discovery
343-
/// formatting child stderr into a frontend-visible error.
344-
pub(crate) fn redact_env_values_in(
345-
s: &str,
346-
env: &std::collections::BTreeMap<String, String>,
347-
) -> String {
348-
let values: Vec<&str> = env
349-
.values()
350-
.filter(|v| !v.is_empty())
351-
.map(String::as_str)
352-
.collect();
353-
redact_secrets_with(s, &values)
354-
}
355-
356340
/// Deploy an agent via provider binary. Returns the provider-assigned agent_id.
357341
///
358342
/// `request_id` is included for provider-side logging/correlation but is not
@@ -584,17 +568,6 @@ mod tests {
584568
);
585569
}
586570

587-
#[test]
588-
fn redact_env_values_in_scrubs_map_values() {
589-
let mut env = std::collections::BTreeMap::new();
590-
env.insert("ANTHROPIC_API_KEY".to_string(), "sk-ant-real".to_string());
591-
env.insert("EMPTY".to_string(), String::new());
592-
let stderr = "auth=sk-ant-real failed; other context";
593-
let r = redact_env_values_in(stderr, &env);
594-
assert!(!r.contains("sk-ant-real"));
595-
assert!(r.contains("[REDACTED]"));
596-
}
597-
598571
#[test]
599572
fn validate_provider_config_rejects_secret_key() {
600573
let cfg = serde_json::json!({"api_key": "val"});

0 commit comments

Comments
 (0)