From b502c6a96d36a97189c164902117626d61c4b7e3 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Thu, 24 Sep 2026 11:26:56 -0400 Subject: [PATCH 1/5] Add Pi harness with provider-aware model discovery Signed-off-by: Salman Mohammed --- crates/agent-controller/src/lib.rs | 3 +- crates/agent-controller/src/pi.rs | 167 ++++++++++ crates/agent-controller/src/runtime.rs | 120 +++++-- crates/agent-controller/src/runtime/tests.rs | 170 ++++++++++ docs/agent-control.md | 72 ++++ src-tauri/src/agent_models.rs | 33 ++ src-tauri/src/agent_models/tests.rs | 36 ++ src-tauri/src/agents.rs | 80 +++-- src-tauri/src/agents/tests.rs | 2 + src-tauri/src/lib.rs | 1 + src-tauri/src/pi_models.rs | 112 +++++++ src-tauri/src/pi_models/tests.rs | 105 ++++++ .../agents/AgentHarnessEditor.test.tsx | 70 ++++ src/bundled/agents/AgentHarnessEditor.tsx | 37 ++- src/bundled/agents/AgentModelPicker.test.tsx | 308 ++++++++++++++++++ src/bundled/agents/AgentModelPicker.tsx | 114 ++++++- src/bundled/agents/AgentSettingsFields.tsx | 19 +- 17 files changed, 1352 insertions(+), 97 deletions(-) create mode 100644 crates/agent-controller/src/pi.rs create mode 100644 src-tauri/src/pi_models.rs create mode 100644 src-tauri/src/pi_models/tests.rs diff --git a/crates/agent-controller/src/lib.rs b/crates/agent-controller/src/lib.rs index bd759f185..b9a03676d 100644 --- a/crates/agent-controller/src/lib.rs +++ b/crates/agent-controller/src/lib.rs @@ -7,6 +7,7 @@ mod create; mod credentials; mod import; mod ownership; +pub mod pi; mod process; mod runtime; mod secret; @@ -17,7 +18,7 @@ pub use config::{AgentEdit, AgentView, ControlSnapshot, HarnessEdit, ProcessStat pub use create::{CreationProfile, NewAgent}; pub use credentials::PlatformCredentials; pub use import::{CredentialedImport, ImportPreview, Imports, LegacySource, PreparedImport}; -pub use runtime::{Action, Controller, GooseModelContext, ModelContext}; +pub use runtime::{installed, Action, Controller, GooseModelContext, ModelContext}; pub use secret::{Credentials, Secret}; pub use store::Store; type Result = std::result::Result; diff --git a/crates/agent-controller/src/pi.rs b/crates/agent-controller/src/pi.rs new file mode 100644 index 000000000..390e285db --- /dev/null +++ b/crates/agent-controller/src/pi.rs @@ -0,0 +1,167 @@ +//! Pi's supported CLI boundary, shared by discovery and ACP launch. +use crate::{ + runtime::{executable, installed}, + HarnessEdit, Result, +}; +use std::{ + collections::BTreeMap, + path::{Path, PathBuf}, +}; + +/// Native only: contains local environment values, never serialized over IPC. +pub struct PiContext { + pub command: PathBuf, + pub workspace: PathBuf, + pub args: Vec, + pub environment: BTreeMap, + pub path: std::ffi::OsString, +} +impl PiContext { + pub(crate) fn new( + harness: &HarnessEdit, + workspace: &str, + environment: &BTreeMap, + ) -> Result { + let adapter = Path::new(&harness.command); + if !adapter.is_absolute() + || adapter.file_name().and_then(|n| n.to_str()) != Some("buzz-pi-acp") + { + return Err("Pi requires an absolute buzz-pi-acp executable path".into()); + } + executable(adapter)?; + if !Path::new(workspace).is_absolute() || !Path::new(workspace).is_dir() { + return Err("Choose an existing absolute workspace before browsing Pi models".into()); + } + crate::config::validate_environment(environment)?; + // The adapter owns its supported runtime flags. Discovery applies its + // narrower, prompt-free contract separately. + let args = &harness.args; + if !args.is_empty() && args.first().map(String::as_str) != Some("--") { + return Err("Pi arguments must follow --".into()); + } + if args.len() > 128 + || args + .iter() + .any(|a| a.is_empty() || a.len() > 8192 || a.contains([',', '\0'])) + { + return Err("Invalid Pi arguments".into()); + } + let resolve = |name| { + let sibling = adapter.parent().unwrap().join(name); + if executable(&sibling).is_ok() { + Some(sibling) + } else { + installed(name) + } + }; + let command = resolve("pi").ok_or("Install Pi and reopen the desktop app")?; + let node = resolve("node").ok_or("Install Node.js for the Pi ACP adapter")?; + let path = std::env::join_paths([ + node.parent().unwrap(), + command.parent().unwrap(), + Path::new("/usr/bin"), + Path::new("/bin"), + Path::new("/usr/sbin"), + Path::new("/sbin"), + ]) + .map_err(|_| "Invalid Pi tools path")?; + let mut environment = environment.clone(); + environment.insert( + "PI_ACP_PI_COMMAND".into(), + command.to_string_lossy().into_owned(), + ); + Ok(Self { + command, + workspace: workspace.into(), + args: args.iter().skip(1).cloned().collect(), + environment, + path, + }) + } + + pub fn catalog_args(&self) -> Result> { + let mut result = Vec::new(); + let mut args = self.args.iter(); + while let Some(arg) = args.next() { + // Match Pi's separate-token CLI syntax exactly. Do not normalize + // options differently from runtime or rebind --api-key to a default + // provider after removing selection for this catalog-only process. + let flag = arg.as_str(); + let selection = matches!(flag, "--provider" | "--model"); + let takes_value = selection + || matches!( + flag, + "--extension" + | "-e" + | "--skill" + | "--prompt-template" + | "--theme" + | "--thinking" + | "--tools" + | "-t" + | "--exclude-tools" + | "-xt" + | "--models" + ); + if takes_value { + let value = args + .next() + .map(String::as_str) + .filter(|v| !v.is_empty() && !v.starts_with('-')) + .ok_or("Pi option is missing a value; check Advanced arguments")?; + if !selection { + result.extend([flag.to_owned(), value.to_owned()]); + } + } else if matches!( + flag, + "--no-extensions" + | "-ne" + | "--no-skills" + | "-ns" + | "--no-prompt-templates" + | "-np" + | "--no-themes" + | "--no-tools" + | "-nt" + | "--no-builtin-tools" + | "-nbt" + | "--no-context-files" + | "-nc" + | "--offline" + | "--approve" + | "-a" + | "--no-approve" + | "-na" + ) { + result.push(arg.clone()); + } else { + return Err("Pi model browsing does not support these Advanced arguments. Runtime arguments are preserved; use a custom model ID or adjust the arguments to browse".into()); + } + } + Ok(result) + } + + pub(crate) fn adapter_args(&self, harness: &HarnessEdit) -> Result> { + if !harness.provider.is_empty() && harness.model.is_empty() { + return Err("Choose a Pi model for the selected provider, or clear both fields to use Pi defaults".into()); + } + for value in [&harness.provider, &harness.model] { + if value.len() > 512 + || value.starts_with('-') + || value.contains([',', '\0']) + || value.chars().any(char::is_control) + { + return Err("Invalid Pi provider or model ID".into()); + } + } + let mut args = vec!["--".into()]; + args.extend(self.args.clone()); + if !harness.provider.is_empty() { + args.extend(["--provider".into(), harness.provider.clone()]); + } + if !harness.model.is_empty() { + args.extend(["--model".into(), harness.model.clone()]); + } + Ok(args) + } +} diff --git a/crates/agent-controller/src/runtime.rs b/crates/agent-controller/src/runtime.rs index 513c866a8..2b4eebb42 100644 --- a/crates/agent-controller/src/runtime.rs +++ b/crates/agent-controller/src/runtime.rs @@ -74,16 +74,29 @@ impl RuntimeBundle { command.env(name, value); } } - let path = std::env::join_paths([ - self.directory.as_path(), - Path::new("/usr/bin"), - Path::new("/bin"), - Path::new("/usr/sbin"), - Path::new("/sbin"), - ]) + let pi = (worker.file_name().and_then(|n| n.to_str()) == Some("buzz-pi-acp")) + .then(|| { + crate::pi::PiContext::new(&agent.harness, &agent.workspace, &agent.environment) + }) + .transpose()?; + let (args, environment, tools_path) = if let Some(pi) = &pi { + ( + pi.adapter_args(&agent.harness)?, + &pi.environment, + pi.path.clone(), + ) + } else { + ( + agent.harness.args.clone(), + &agent.environment, + "/usr/bin:/bin:/usr/sbin:/sbin".into(), + ) + }; + let path = std::env::join_paths( + std::iter::once(self.directory.clone()).chain(std::env::split_paths(&tools_path)), + ) .map_err(|_| "Invalid runtime tools path")?; - command.envs(&agent.environment); - command.env("PATH", path); + command.envs(environment).env("PATH", path); let key_hex = key.hex(); command .env("BUZZ_PRIVATE_KEY", &*key_hex) @@ -91,7 +104,7 @@ impl RuntimeBundle { .env("BUZZ_RELAY_URL", &agent.relay_url) .env("BUZZ_AUTH_TAG", agent.auth_tag.as_deref().unwrap_or("")) .env("BUZZ_ACP_AGENT_COMMAND", worker) - .env("BUZZ_ACP_AGENT_ARGS", agent.harness.args.join(",")) + .env("BUZZ_ACP_AGENT_ARGS", args.join(",")) .env("BUZZ_ACP_SYSTEM_PROMPT", &agent.system_prompt) .env("BUZZ_ACP_DISPLAY_NAME", &agent.name) .env("BUZZ_ACP_LAZY_POOL", "true") @@ -111,6 +124,7 @@ impl RuntimeBundle { let mapping = match worker_name { "buzz-agent" => Some(("BUZZ_AGENT_MODEL", "BUZZ_AGENT_PROVIDER")), "goose" => Some(("GOOSE_MODEL", "GOOSE_PROVIDER")), + "buzz-pi-acp" => None, _ if !agent.harness.provider.is_empty() => return Err("Set provider configuration through this external harness's environment; a provider selector mapping is not available".into()), _ => None, }; @@ -136,6 +150,11 @@ impl RuntimeBundle { } } if let Some(value) = model { + let value = if pi.is_some() && !agent.harness.provider.is_empty() { + format!("{}/{value}", agent.harness.provider) + } else { + value.to_owned() + }; command.env("BUZZ_ACP_MODEL", value); } if respond_to == "allowlist" { @@ -211,6 +230,24 @@ fn effective_databricks(agent: &Agent) -> Result Option { + let mut dirs = Vec::new(); + if let Some(home) = std::env::var_os("HOME") { + dirs.push(PathBuf::from(home).join(".local/bin")); + } + dirs.extend(std::env::split_paths( + &std::env::var_os("PATH").unwrap_or_default(), + )); + dirs.extend([ + PathBuf::from("/opt/homebrew/bin"), + PathBuf::from("/usr/local/bin"), + ]); + dirs.into_iter() + .filter(|p| p.is_absolute()) + .map(|p| p.join(name)) + .find(|p| executable(p).is_ok()) +} + pub(crate) fn executable(path: &Path) -> Result<()> { let metadata = path .metadata() @@ -321,20 +358,7 @@ impl Controller { /// Native-only catalog configuration. Never serialize environment values or /// lend runtime credentials to model discovery. Resolve an unsaved edit on a /// clone using the same validation and precedence as Save/runtime. - pub fn model_context(&self, id: &str, revision: u64, edit: AgentEdit) -> Result { - let agent = self.edited_model_agent(id, revision, edit)?; - model_context(&agent.harness, &agent.environment) - } - pub fn goose_model_context( - &self, - id: &str, - revision: u64, - edit: AgentEdit, - ) -> Result { - let agent = self.edited_model_agent(id, revision, edit)?; - goose_model_context(&agent.harness, &agent.environment) - } - fn edited_model_agent(&self, id: &str, revision: u64, edit: AgentEdit) -> Result { + fn edited_agent(&self, id: &str, revision: u64, edit: AgentEdit) -> Result { let mut agent = self .store .agents()? @@ -349,20 +373,41 @@ impl Controller { agent.apply(edit)?; Ok(agent) } + pub fn model_context(&self, id: &str, revision: u64, edit: AgentEdit) -> Result { + let agent = self.edited_agent(id, revision, edit)?; + model_context(&agent.harness, &agent.environment) + } + pub fn goose_model_context( + &self, + id: &str, + revision: u64, + edit: AgentEdit, + ) -> Result { + let agent = self.edited_agent(id, revision, edit)?; + goose_model_context(&agent.harness, &agent.environment) + } + pub fn pi_model_context( + &self, + id: &str, + revision: u64, + edit: AgentEdit, + ) -> Result { + let agent = self.edited_agent(id, revision, edit)?; + crate::pi::PiContext::new(&agent.harness, &agent.workspace, &agent.environment) + } + pub fn draft_pi_model_context(edit: AgentEdit) -> Result { + crate::pi::PiContext::new( + &edit.harness, + &edit.workspace, + &draft_environment(edit.environment), + ) + } pub fn draft_goose_model_context(edit: AgentEdit) -> Result { - let environment = edit - .environment - .into_iter() - .filter_map(|(key, value)| value.map(|value| (key, value))) - .collect(); + let environment = draft_environment(edit.environment); goose_model_context(&edit.harness, &environment) } pub fn draft_model_context(edit: AgentEdit) -> Result { - let environment = edit - .environment - .into_iter() - .filter_map(|(key, value)| value.map(|value| (key, value))) - .collect(); + let environment = draft_environment(edit.environment); model_context(&edit.harness, &environment) } pub fn requires_legacy_handover(&self, id: &str) -> Result { @@ -605,6 +650,13 @@ impl Drop for Controller { #[cfg(test)] mod tests; +fn draft_environment(patch: BTreeMap>) -> BTreeMap { + patch + .into_iter() + .filter_map(|(key, value)| value.map(|v| (key, v))) + .collect() +} + fn model_context( harness: &crate::HarnessEdit, environment: &BTreeMap, diff --git a/crates/agent-controller/src/runtime/tests.rs b/crates/agent-controller/src/runtime/tests.rs index 40a1af144..f94d47714 100644 --- a/crates/agent-controller/src/runtime/tests.rs +++ b/crates/agent-controller/src/runtime/tests.rs @@ -715,3 +715,173 @@ fn goose_model_context_uses_effective_draft_provider_without_projecting_secrets( assert!(!context.environment.contains_key("GOOSE_PROVIDER")); assert!(Controller::draft_goose_model_context(edit(Some("openai"))).is_err()); } + +#[test] +#[cfg(unix)] +fn pi_selection_and_extensions_survive_save_reopen_and_reach_adapter() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let tools = tempfile::tempdir().unwrap(); + let runtime = bundle(tools.path()); + let adapter = tools.path().join("buzz-pi-acp"); + fs::write(&adapter, "#!/bin/sh\nexit 0\n").unwrap(); + fs::set_permissions(&adapter, fs::Permissions::from_mode(0o700)).unwrap(); + let extension = tools.path().join("extension with spaces.ts"); + fs::write(&extension, "export default function() {};").unwrap(); + let mut a = agent(dir.path()); + a.harness.command = adapter.display().to_string(); + a.harness.provider = "custom".into(); + a.harness.model = "namespace/exact.id".into(); + a.harness.args = vec![ + "--".into(), + "--extension".into(), + extension.display().to_string(), + ]; + for tool in ["pi", "node"] { + fs::copy(&adapter, tools.path().join(tool)).unwrap(); + } + a.environment.insert( + "PI_CODING_AGENT_DIR".into(), + dir.path().display().to_string(), + ); + let root = dir.path().join("config"); + Store::open(root.clone()) + .unwrap() + .insert(vec![a.clone()]) + .unwrap(); + let store = Store::open(root).unwrap(); + let saved = store.agents().unwrap().remove(0); + let key = Secret::parse(KEY, PUB).unwrap(); + let command = runtime.command(&saved, &key).unwrap(); + let env: BTreeMap<_, _> = command + .get_envs() + .map(|(k, v)| (k.to_str().unwrap(), v.unwrap().to_str().unwrap())) + .collect(); + assert_eq!(env["BUZZ_ACP_MODEL"], "custom/namespace/exact.id"); + assert_eq!( + env["BUZZ_ACP_AGENT_ARGS"], + format!( + "--,--extension,{},--provider,custom,--model,namespace/exact.id", + extension.display() + ) + ); + assert!(!env.contains_key("GOOSE_MODEL")); + let controller = Controller::new( + store, + Arc::new(Memory), + Ok(runtime), + dir.path().join("ownership"), + ); + let edit = AgentEdit { + name: a.name.clone(), + system_prompt: a.system_prompt.clone(), + workspace: a.workspace.clone(), + harness: a.harness.clone(), + environment: BTreeMap::new(), + }; + let context = controller.pi_model_context(&a.id, 1, edit.clone()).unwrap(); + assert_eq!(context.args, a.harness.args[1..]); + assert_eq!( + context.environment["PI_CODING_AGENT_DIR"], + dir.path().display().to_string() + ); + assert!(controller.pi_model_context(&a.id, 2, edit.clone()).is_err()); + let mut patch = edit.clone(); + patch.environment.insert( + "PI_CODING_AGENT_DIR".into(), + Some("/override/config".into()), + ); + let context = controller + .pi_model_context(&a.id, 1, patch.clone()) + .unwrap(); + assert_eq!( + context.environment["PI_CODING_AGENT_DIR"], + "/override/config" + ); + patch.environment.insert("PI_CODING_AGENT_DIR".into(), None); + assert!(!controller + .pi_model_context(&a.id, 1, patch) + .unwrap() + .environment + .contains_key("PI_CODING_AGENT_DIR")); + // Draft resolution did not mutate the saved override. + assert_eq!( + controller.store.agents().unwrap()[0].environment["PI_CODING_AGENT_DIR"], + a.environment["PI_CODING_AGENT_DIR"] + ); + let mut configured = saved.clone(); + configured.harness.model.clear(); + assert!(controller + .bundle + .as_ref() + .unwrap() + .command(&configured, &key) + .unwrap_err() + .contains("Choose a Pi model")); + configured.harness.provider.clear(); + configured.harness.args = vec![ + "--".into(), + "--thinking".into(), + "high".into(), + "--skill".into(), + "/local/skill".into(), + "--tools".into(), + "read".into(), + ]; + let launch = controller + .bundle + .as_ref() + .unwrap() + .command(&configured, &key) + .unwrap(); + assert_eq!( + launch + .get_envs() + .find(|(k, _)| *k == "BUZZ_ACP_AGENT_ARGS") + .unwrap() + .1 + .unwrap(), + "--,--thinking,high,--skill,/local/skill,--tools,read" + ); + let mut advanced = edit; + advanced.harness.args = configured.harness.args.clone(); + let context = Controller::draft_pi_model_context(advanced.clone()).unwrap(); + assert_eq!( + context.catalog_args().unwrap(), + configured.harness.args[1..] + ); + advanced.harness.args = vec![ + "--".into(), + "--provider".into(), + "old".into(), + "--model".into(), + "invalid".into(), + ]; + assert!(Controller::draft_pi_model_context(advanced.clone()) + .unwrap() + .catalog_args() + .unwrap() + .is_empty()); + for unsupported in [ + vec!["--custom-extension-flag"], + vec!["--extension=/local/extension.ts"], + vec!["--api-key", "synthetic-key"], + vec!["a prompt"], + ] { + advanced.harness.args = std::iter::once("--") + .chain(unsupported) + .map(str::to_owned) + .collect(); + assert!(Controller::draft_pi_model_context(advanced.clone()) + .unwrap() + .catalog_args() + .is_err()); + configured.harness.args = advanced.harness.args.clone(); + assert!(controller + .bundle + .as_ref() + .unwrap() + .command(&configured, &key) + .is_ok()); + } +} diff --git a/docs/agent-control.md b/docs/agent-control.md index 0beb7bcb2..eee1fc101 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -347,3 +347,75 @@ import can leave create-only app custody for retry but no enabled/configured age No remote/team/mesh runtime or conditional attestation is added. Synthetic checks do not establish actual Keychain ACLs, production TLS/inference, live replies, forced native quit, signed packaging or other-platform behavior. + +## Pi harness + +Install Pi, Node.js, and the `buzz-pi-acp` adapter, then reopen the desktop app. +Pi appears alongside Buzz Agent and Goose. Availability means the executables +were found, not that authentication or inference has been verified. This +integration uses the adapter's Pi argument forwarding after `--` (verified with +buzz-pi-acp 0.0.33) and Pi's `get_available_models` RPC (verified with Pi 0.86.1). + +Choose **Pi → LLM Provider → Browse models**, or leave Provider unset and Browse +to see all locally available providers. The returned provider/model pair is saved +as separate fields; model IDs retain namespace slashes and punctuation. Browse +also adds extension-provided providers to the provider choices. Pi provider +changes filter the loaded catalog without launching another lookup. Workspace, +arguments or environment changes retire it and clear discovered provider suggestions. Custom provider +and model entry remain available, including when lookup fails or returns no models. +When a provider is set, enter its exact model ID without adding the provider prefix. +The Advanced model field preserves text literally, including IDs that themselves +start with the provider name. After Browse, an unlisted ID carries a warning; +manual IDs remain allowed and an available catalog is not inference validation. +Clear both fields to keep Pi's own defaults. Choosing a provider requires a model +before Start; Pi otherwise silently ignores a provider-only flag. Save does not restart a running agent; +use Restart explicitly to apply changes. + +Discovery launches the same locally resolved Pi used by the ACP adapter, in the +agent's workspace, with the same explicit environment and extension arguments. +It uses `--mode rpc --no-session --no-themes` and sends only +`get_available_models`, never a prompt or Buzz identity. Selection is omitted +from catalog startup so a stale model cannot prevent finding its replacement. +Cancel, changed workspace/configuration, and closing the editor retire the native +lookup; Unix cleanup kills the lookup process group. Errors require explicit retry. +Pi may return a cached extension catalog; Refresh reloads Pi’s available snapshot +and does not guarantee a fresh remote catalog. The catalog reflects Pi's available models and local credentials, not a guarantee +of inference permission. Authentication and extension caches remain Pi-owned; +configure sign-in in Pi. Extensions are executable local code and can perform +their own initialization/authentication during discovery. + +Runtime forwards Provider and Model as `--provider` / `--model` to the adapter +and uses `provider/exact-id` for ACP model selection. There are no invented Pi +provider/model environment variables. The controller owns `PI_ACP_PI_COMMAND`; +it resolves Pi and Node beside the adapter first, then the usual local install +locations. Ambient provider credentials are not inherited. Use Pi's credential +store or explicit write-only per-agent environment patches. + +Optional extension configuration uses Pi's existing facilities, with no provider +package bundled into the OSS app: + +- Install/configure packages in Pi normally; both discovery and runtime load them. +- Set `PI_CODING_AGENT_DIR` in Advanced environment to use a specific local Pi + configuration directory. It is saved locally, never projected in a snapshot. +- To load a particular extension, set Advanced arguments to + `["--", "--extension", "/absolute/path/to/extension.ts"]`. Paths containing spaces + are supported. `--no-extensions` disables automatic extension discovery while + keeping explicitly supplied extensions. Advanced runtime arguments are preserved + and forwarded to the adapter, including + thinking, skills and tools. Browse supports standard Pi configuration options + and strips provider/model flags for catalog startup. Unsupported extension flags + or positional input block Browse with an explanation, while manual entry and + runtime arguments remain available. Browse also rejects inline `--flag=value` + syntax and `--api-key`, whose meaning depends on the selected startup provider; + use Pi's local credential store or explicit provider environment instead. + The adapter still owns its reserved + session, prompt and mode flags. Explicit Provider/Model fields are appended last. + +Internal distributions can provision a pinned extension package and Pi config, +or supply an installed extension path through these same settings. Keep private +package URLs, hosts, filters, authentication and model policy in the private +packaging/configuration owner. Discovery and runtime must point at that same +configuration. The current internal release repository builds the old desktop; +its generic build environment injection is not a Pi resource-bundling contract +for this app. Signed bundling, automatic employee provisioning and release +pipeline migration require separate release work; no release is published here. diff --git a/src-tauri/src/agent_models.rs b/src-tauri/src/agent_models.rs index 483a3d4c9..4c2370086 100644 --- a/src-tauri/src/agent_models.rs +++ b/src-tauri/src/agent_models.rs @@ -248,6 +248,39 @@ pub(crate) async fn agent_models_run( ) -> Result { let host = state.inner().clone(); let controller = agents.inner().clone(); + if request.edit.as_ref().is_some_and(|e| { + std::path::Path::new(&e.harness.command) + .file_name() + .and_then(|n| n.to_str()) + == Some("buzz-pi-acp") + }) { + let prepared = controller.pi_model_context( + request.id.as_deref(), + request.expected_revision, + request.edit.clone().unwrap(), + ); + return host + .run(ticket, async move { + if request.action == Operation::Disconnect { + return Err("Pi credentials are managed by Pi".into()); + } + let models = crate::pi_models::fetch(prepared?) + .await? + .into_iter() + .map(|id| Model { + name: id.clone(), + id, + }) + .collect(); + Ok(Catalog { + host: String::new(), + models, + model_overridden: false, + disconnected: false, + }) + }) + .await; + } let goose = request.edit.as_ref().is_some_and(|edit| { std::path::Path::new(&edit.harness.command) .file_name() diff --git a/src-tauri/src/agent_models/tests.rs b/src-tauri/src/agent_models/tests.rs index 185c71f80..27d3ffbf9 100644 --- a/src-tauri/src/agent_models/tests.rs +++ b/src-tauri/src/agent_models/tests.rs @@ -515,3 +515,39 @@ async fn unstarted_ticket_expires_and_old_run_cannot_claim_its_replacement() { host.cancel(next).unwrap(); assert!(host.begin().is_ok()); } + +#[cfg(unix)] +#[test] +fn pi_catalog_uses_native_ticket_and_draft_configuration_without_saving() { + use std::os::unix::fs::PermissionsExt; + let (dir, _host, _app, view) = fixture(); + std::fs::create_dir(dir.path().join("local-config")).unwrap(); + let tools = dir.path().join("tools"); + std::fs::create_dir(&tools).unwrap(); + for tool in ["pi", "node", "buzz-pi-acp"] { + let file = tools.join(tool); + std::fs::write(&file, r#"#!/bin/sh +read request +[ "$PI_CODING_AGENT_DIR" -ef "./local-config" ] || exit 1 +[ "$BUZZ_PRIVATE_KEY" = "" ] || exit 1 +printf '%s\n' '{"id":"catalog","type":"response","command":"get_available_models","success":true,"data":{"models":[{"provider":"extension","id":"namespace/model.v1"}]}}' +"#).unwrap(); + std::fs::set_permissions(file, std::fs::Permissions::from_mode(0o700)).unwrap(); + } + let ticket = invoke(&view, "agent_models_begin", json!({})).unwrap(); + let result=invoke(&view,"agent_models_run",json!({"ticket":ticket,"request":{ + "host":"","filter":"","action":"connect","edit":{ + "name":"Pi draft","systemPrompt":"","workspace":dir.path(), + "harness":{"command":tools.join("buzz-pi-acp"),"args":[],"provider":"extension","model":"invalid-old-id"}, + "environment":{"PI_CODING_AGENT_DIR":dir.path().join("local-config")} + } + }})).unwrap(); + assert_eq!( + result["models"], + json!([{"id":"extension/namespace/model.v1","name":"extension/namespace/model.v1"}]) + ); + assert_eq!( + invoke(&view, "agent_control_snapshot", json!({})).unwrap()["agents"], + json!([]) + ); +} diff --git a/src-tauri/src/agents.rs b/src-tauri/src/agents.rs index ef6f21e38..6be5f2d21 100644 --- a/src-tauri/src/agents.rs +++ b/src-tauri/src/agents.rs @@ -111,6 +111,10 @@ const GOOSE_PROVIDERS: &[ProviderOption] = &[ fn harness_options() -> Vec { let goose = installed_goose(); + let pi = buzz_agent_controller::installed("buzz-pi-acp"); + let pi_available = pi.is_some() + && buzz_agent_controller::installed("pi").is_some() + && buzz_agent_controller::installed("node").is_some(); vec![ HarnessOption { command: "buzz-agent".into(), @@ -132,42 +136,42 @@ fn harness_options() -> Vec { default_args: &["acp"], providers: GOOSE_PROVIDERS, }, + HarnessOption { + command: pi.map_or_else( + || "buzz-pi-acp".into(), + |p| p.to_string_lossy().into_owned(), + ), + label: "Pi", + available: pi_available, + default_args: &[], + providers: &[ + ProviderOption { + value: "anthropic", + label: "Anthropic", + }, + ProviderOption { + value: "openai", + label: "OpenAI", + }, + ProviderOption { + value: "openai-codex", + label: "OpenAI Codex", + }, + ProviderOption { + value: "google", + label: "Google", + }, + ProviderOption { + value: "openrouter", + label: "OpenRouter", + }, + ], + }, ] } fn installed_goose() -> Option { - let mut directories = Vec::new(); - if let Some(home) = std::env::var_os("HOME") { - directories.push(PathBuf::from(home).join(".local/bin")); - } - directories.extend(std::env::split_paths( - &std::env::var_os("PATH").unwrap_or_default(), - )); - directories.extend([ - PathBuf::from("/opt/homebrew/bin"), - PathBuf::from("/usr/local/bin"), - ]); - directories - .into_iter() - .filter(|dir| dir.is_absolute()) - .map(|dir| dir.join("goose")) - .find(|path| { - let Ok(metadata) = path.metadata() else { - return false; - }; - if !metadata.is_file() { - return false; - } - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - metadata.permissions().mode() & 0o111 != 0 - } - #[cfg(not(unix))] - { - true - } - }) + buzz_agent_controller::installed("goose") } struct Host { @@ -330,6 +334,18 @@ impl AgentHost { _ => Err("Invalid agent model context".into()), }) } + pub(crate) fn pi_model_context( + &self, + id: Option<&str>, + revision: Option, + edit: AgentEdit, + ) -> Result { + self.with(|host| match (id, revision) { + (Some(id), Some(revision)) => host.controller.pi_model_context(id, revision, edit), + (None, None) => Controller::draft_pi_model_context(edit), + _ => Err("Invalid agent model context".into()), + }) + } pub(crate) fn shutdown(&self) -> Result<(), String> { self.1.store(true, Ordering::SeqCst); let mut state = self diff --git a/src-tauri/src/agents/tests.rs b/src-tauri/src/agents/tests.rs index e8385bd42..3161c66b4 100644 --- a/src-tauri/src/agents/tests.rs +++ b/src-tauri/src/agents/tests.rs @@ -119,6 +119,8 @@ fn real_ipc_snapshot_save_cas_stop_and_launch_gate() { "providers":[{"value":"databricks_v2", "label":"Databricks v2"}] }) ); + assert_eq!(before["harnessOptions"][2]["label"], "Pi"); + assert_eq!(before["harnessOptions"][2]["defaultArgs"], json!([])); assert_eq!(before["harnessOptions"][1]["label"], "Goose"); assert_eq!(before["harnessOptions"][1]["defaultArgs"], json!(["acp"])); assert_eq!( diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index f58b534a7..6bdb36b4a 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -5,6 +5,7 @@ mod notifications; mod terminal; use agent_models::{agent_models_begin, agent_models_cancel, agent_models_run, ModelHost}; mod goose_models; +mod pi_models; use agents::{ agent_control_action, agent_control_create_commit, agent_control_create_prepare, agent_control_creation_profile, agent_control_import_commit, agent_control_import_preview, diff --git a/src-tauri/src/pi_models.rs b/src-tauri/src/pi_models.rs new file mode 100644 index 000000000..fab430563 --- /dev/null +++ b/src-tauri/src/pi_models.rs @@ -0,0 +1,112 @@ +//! Ephemeral Pi RPC catalog. No Buzz identity, prompt, or saved Pi session. +use buzz_agent_controller::pi::PiContext; +use serde_json::{json, Value}; +use std::process::Stdio; +use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; + +const FAILURE: &str = "Pi models unavailable. Check Pi sign-in and extension configuration, then retry or enter a custom ID"; + +// Extensions may spawn helpers. Cancellation must retire the whole lookup group. +struct LookupChild(tokio::process::Child); +impl Drop for LookupChild { + fn drop(&mut self) { + #[cfg(unix)] + if let Some(pid) = self.0.id() { + unsafe { + libc::kill(-(pid as i32), libc::SIGKILL); + } + } + let _ = self.0.start_kill(); + } +} + +pub(super) async fn fetch(context: PiContext) -> Result, String> { + let mut command = tokio::process::Command::new(&context.command); + command + .args(["--mode", "rpc", "--no-session", "--no-themes"]) + .args(context.catalog_args()?) + .current_dir(&context.workspace) + .env_clear() + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .kill_on_drop(true); + for key in [ + "HOME", + "TMPDIR", + "USER", + "LOGNAME", + "LANG", + "SSL_CERT_FILE", + "SSL_CERT_DIR", + ] { + if let Some(value) = std::env::var_os(key) { + command.env(key, value); + } + } + command.envs(context.environment).env("PATH", context.path); + #[cfg(unix)] + command.process_group(0); + let mut child = LookupChild( + command + .spawn() + .map_err(|_| "Could not start Pi to list models")?, + ); + let mut stdin = child.0.stdin.take().ok_or(FAILURE)?; + stdin + .write_all(b"{\"id\":\"catalog\",\"type\":\"get_available_models\"}\n") + .await + .map_err(|_| FAILURE)?; + let stdout = child.0.stdout.take().ok_or(FAILURE)?; + let mut reader = BufReader::new(stdout.take(8 * 1024 * 1024 + 1)); + let result = tokio::time::timeout(std::time::Duration::from_secs(60), async { + for _ in 0..100 { + let mut line = String::new(); + if reader.read_line(&mut line).await.map_err(|_| FAILURE)? == 0 { + break; + } + let value: Value = serde_json::from_str(&line).map_err(|_| FAILURE)?; + if value.get("id") == Some(&json!("catalog")) { + return parse_response(&value); + } + } + Err(FAILURE.into()) + }) + .await + .map_err(|_| "Pi model lookup timed out; retry explicitly")?; + drop(stdin); + // Drop kills helpers too, even after a successful response. + drop(child); + result +} +fn parse_response(value: &Value) -> Result, String> { + if value["type"] != "response" + || value["command"] != "get_available_models" + || value["success"] != true + { + return Err(FAILURE.into()); + } + let models = value["data"]["models"].as_array().ok_or(FAILURE)?; + if models.len() > 10_000 { + return Err("Pi model catalog is too large".into()); + } + let mut result = Vec::new(); + for model in models { + let provider = model["provider"].as_str().ok_or(FAILURE)?; + let id = model["id"].as_str().ok_or(FAILURE)?; + if [provider, id] + .iter() + .any(|s| s.is_empty() || s.len() > 512 || s.chars().any(char::is_control)) + || provider.contains('/') + { + return Err("Pi returned an invalid model ID".into()); + } + result.push(format!("{provider}/{id}")); + } + result.sort(); + result.dedup(); + Ok(result) +} + +#[cfg(test)] +mod tests; diff --git a/src-tauri/src/pi_models/tests.rs b/src-tauri/src/pi_models/tests.rs new file mode 100644 index 000000000..28c6e51ea --- /dev/null +++ b/src-tauri/src/pi_models/tests.rs @@ -0,0 +1,105 @@ +use super::*; +#[test] +fn catalog_keeps_exact_ids_and_provider_boundaries_and_redacts_errors() { + let response = json!({"type":"response","command":"get_available_models","success":true,"data":{"models":[{"provider":"custom","id":"namespace/model.v1"}]}}); + assert_eq!( + parse_response(&response).unwrap(), + ["custom/namespace/model.v1"] + ); + for bad in [ + json!({"success":false,"error":"secret"}), + json!({"type":"response","command":"get_available_models","success":true,"data":{"models":[{"provider":"bad/provider","id":"model"}]}}), + ] { + let error = parse_response(&bad).unwrap_err(); + assert!(!error.contains("secret")); + } +} +#[cfg(unix)] +fn fixture(script: &str) -> (tempfile::TempDir, PiContext) { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let command = dir.path().join("pi"); + std::fs::write(&command, format!("#!/bin/sh\n{script}")).unwrap(); + std::fs::set_permissions(&command, std::fs::Permissions::from_mode(0o700)).unwrap(); + let context = PiContext { + command, + workspace: dir.path().into(), + args: vec![], + environment: Default::default(), + path: "/usr/bin:/bin".into(), + }; + (dir, context) +} +#[cfg(unix)] +#[tokio::test] +async fn reads_rpc_and_reports_exit_failure() { + let (_dir, context) = fixture("read request\ncase \"$request\" in *get_available_models*) printf '%s\\n' '{\"id\":\"catalog\",\"type\":\"response\",\"command\":\"get_available_models\",\"success\":true,\"data\":{\"models\":[{\"provider\":\"p\",\"id\":\"exact/id\"}]}}';; esac\n"); + assert_eq!(fetch(context).await.unwrap(), ["p/exact/id"]); + let (_dir, context) = fixture("exit 1\n"); + assert!(fetch(context).await.is_err()); +} +#[cfg(unix)] +#[tokio::test] +async fn cancellation_kills_lookup_after_observed_start() { + let (dir, context) = fixture("sleep 300 &\nhelper=$!\nprintf '%s %s\\n' \"$$\" \"$helper\" > started\nread request\nwait\n"); + let task = tokio::spawn(fetch(context)); + let pid = tokio::time::timeout(std::time::Duration::from_secs(5), async { + loop { + if let Ok(text) = std::fs::read_to_string(dir.path().join("started")) { + let pids: Vec = text + .split_whitespace() + .filter_map(|v| v.parse().ok()) + .collect(); + if pids.len() == 2 { + break pids; + } + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while pid.iter().any(|pid| unsafe { libc::kill(*pid, 0) } == 0) { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); +} + +#[tokio::test] +#[ignore = "requires explicitly selected installed Pi/ACP and local configuration; no inference"] +async fn installed_pi_catalog_uses_production_context() { + use buzz_agent_controller::{AgentEdit, Controller, HarnessEdit}; + use std::collections::BTreeMap; + let adapter = std::env::var("BUZZ_TEST_PI_ADAPTER").expect("set BUZZ_TEST_PI_ADAPTER"); + let dir = tempfile::tempdir().unwrap(); + let context = Controller::draft_pi_model_context(AgentEdit { + name: "Probe".into(), + system_prompt: String::new(), + workspace: dir.path().display().to_string(), + harness: HarnessEdit { + command: adapter, + args: vec!["--".into(), "--thinking".into(), "high".into()], + model: String::new(), + provider: String::new(), + databricks: None, + }, + environment: BTreeMap::new(), + }) + .unwrap(); + let models = fetch(context).await.unwrap(); + assert!(!models.is_empty()); + println!( + "Production Pi catalog: {} models, {} providers", + models.len(), + models + .iter() + .filter_map(|m| m.split('/').next()) + .collect::>() + .len() + ); +} diff --git a/src/bundled/agents/AgentHarnessEditor.test.tsx b/src/bundled/agents/AgentHarnessEditor.test.tsx index 536331889..1253e9922 100644 --- a/src/bundled/agents/AgentHarnessEditor.test.tsx +++ b/src/bundled/agents/AgentHarnessEditor.test.tsx @@ -123,3 +123,73 @@ it("preserves Goose settings while editing a custom executable path", async () = model: "m", }); }); + +it("switching Pi, Goose and Buzz resets incompatible selections and uses each harness arguments", async () => { + let current = { + ...agentDraft(controlFixture().agent), + command: "buzz-agent", + provider: "databricks_v2", + model: "old", + args: "[]", + }; + function Editor() { + const [draft, setDraft] = useState(current); + current = draft; + return ( + setDraft((d) => ({ ...d, ...patch }))} + /> + ); + } + const user = userEvent.setup(); + render(); + await user.click(screen.getByRole("combobox", { name: "Harness" })); + await user.click(await screen.findByRole("option", { name: "Pi" })); + expect(current).toMatchObject({ + command: "/local/buzz-pi-acp", + args: "[]", + provider: "", + model: "", + }); + await user.click(screen.getByRole("combobox", { name: "LLM Provider" })); + await user.click(await screen.findByRole("option", { name: "extension" })); + expect(current.provider).toBe("extension"); + await user.click(screen.getByRole("combobox", { name: "Harness" })); + await user.click(await screen.findByRole("option", { name: "Goose" })); + expect(current).toMatchObject({ + command: "/local/goose", + args: '["acp"]', + provider: "", + model: "", + }); + await user.click(screen.getByRole("combobox", { name: "Harness" })); + await user.click(await screen.findByRole("option", { name: "Buzz Agent" })); + expect(current).toMatchObject({ + command: "buzz-agent", + args: "[]", + provider: "databricks_v2", + model: "", + }); +}); diff --git a/src/bundled/agents/AgentHarnessEditor.tsx b/src/bundled/agents/AgentHarnessEditor.tsx index e6f563f6a..ce262d553 100644 --- a/src/bundled/agents/AgentHarnessEditor.tsx +++ b/src/bundled/agents/AgentHarnessEditor.tsx @@ -9,19 +9,21 @@ import { isGoose, type AgentDraft } from "./agent-edit"; export function AgentHarnessEditor({ draft, options, + piProviders = [], onChange, disabled = false, }: { draft: AgentDraft; + piProviders?: string[]; options: NonNullable; disabled?: boolean; onChange(patch: Partial): void; }) { + const executable = draft.command.split("/").at(-1); const harness = options.find((option) => option.command === draft.command) ?? - (isGoose(draft.command) - ? options.find((option) => isGoose(option.command)) - : undefined); + options.find((option) => option.command.split("/").at(-1) === executable); + const external = harness?.label === "Goose" || harness?.label === "Pi"; return (
{ const option = options.find((item) => item.command === command); - const enteringGoose = isGoose(command); - const leavingGoose = isGoose(draft.command); + const enteringExternal = + option?.label === "Goose" || option?.label === "Pi"; onChange({ command, - ...(pickedOption && (enteringGoose || leavingGoose) + ...(pickedOption && (enteringExternal || external) ? { args: JSON.stringify(option?.defaultArgs ?? []), - provider: enteringGoose + provider: enteringExternal ? "" : (option?.providers[0]?.value ?? ""), model: "", @@ -60,20 +62,37 @@ export function AgentHarnessEditor({ Install the Goose CLI to use it as a harness.

)} + {options.some( + (option) => option.label === "Pi" && option.available === false, + ) && ( +

+ Install Pi, buzz-pi-acp and Node.js, then reopen the desktop app to + use Pi. +

+ )} + !harness.providers.some((option) => option.value === p), + ) + .map((value) => ({ value, label: value })) + : []), ]} onChange={(provider) => onChange({ provider, - ...(isGoose(draft.command) ? { model: "" } : {}), + ...(external ? { model: "" } : {}), }) } /> diff --git a/src/bundled/agents/AgentModelPicker.test.tsx b/src/bundled/agents/AgentModelPicker.test.tsx index d2e2d7c9d..b35f4169e 100644 --- a/src/bundled/agents/AgentModelPicker.test.tsx +++ b/src/bundled/agents/AgentModelPicker.test.tsx @@ -157,3 +157,311 @@ for (const opening of ["typing", "ArrowDown", "closed"] as const) { } }); } + +it("Pi discovers extension providers before start and selects the exact provider/model pair with one Browse", async () => { + const f = controlFixture(); + let release!: (value: { + host: string; + models: { id: string; name: string }[]; + modelOverridden: boolean; + disconnected: boolean; + }) => void; + const run = vi.fn( + () => + new Promise[0]>((resolve) => { + release = resolve; + }), + ); + f.host.models = { begin: async () => 1, run, cancel: async () => {} }; + const control = createAgentControl(f.host); + const providers = vi.fn(); + const user = userEvent.setup(); + let current = { + ...agentDraft(f.agent), + command: "/local/buzz-pi-acp", + args: "[]", + provider: "", + model: "", + }; + function Editor() { + const [draft, setDraft] = useState(current); + current = draft; + return ( + setDraft((d) => ({ ...d, ...patch }))} + /> + ); + } + const view = render(); + try { + await user.click(screen.getByRole("button", { name: "Browse models" })); + await waitFor(() => expect(run).toHaveBeenCalledOnce()); + expect( + screen.getByRole("button", { name: "Cancel model lookup", hidden: true }), + ).toBeVisible(); + await act(async () => + release({ + host: "", + models: [ + { + id: "extension/namespace/model.v1", + name: "extension/namespace/model.v1", + }, + ], + modelOverridden: false, + disconnected: false, + }), + ); + await user.click( + await screen.findByRole("option", { + name: /extension\/namespace\/model.v1/, + }), + ); + expect(current.provider).toBe("extension"); + expect(current.model).toBe("namespace/model.v1"); + expect(providers).toHaveBeenCalledWith(["extension"]); + await user.click(screen.getByRole("button", { name: "Browse models" })); + expect( + await screen.findByRole("option", { + name: /extension\/namespace\/model.v1/, + }), + ).toBeVisible(); + expect(run).toHaveBeenCalledOnce(); + } finally { + view.unmount(); + control.dispose(); + } +}); + +it("Pi cancellation and workspace changes reject late catalogs; explicit retry recovers", async () => { + const f = controlFixture(); + let release!: (value: { + host: string; + models: { id: string; name: string }[]; + modelOverridden: boolean; + disconnected: boolean; + }) => void; + const run = vi.fn( + () => + new Promise[0]>((resolve) => { + release = resolve; + }), + ); + const cancel = vi.fn(async () => {}); + f.host.models = { begin: async () => 1, run, cancel }; + const control = createAgentControl(f.host); + const user = userEvent.setup(); + let draft = { + ...agentDraft(f.agent), + command: "/local/buzz-pi-acp", + args: "[]", + provider: "custom", + model: "kept", + }; + const renderPicker = () => ( + {}} + /> + ); + const view = render(renderPicker()); + const result = { + host: "", + models: [{ id: "custom/new", name: "custom/new" }], + modelOverridden: false, + disconnected: false, + }; + try { + await user.click(screen.getByRole("button", { name: "Browse models" })); + await waitFor(() => expect(run).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(screen.getByRole("combobox", { name: "Model" })).toHaveAttribute( + "aria-expanded", + "true", + ), + ); + await user.keyboard("{Escape}"); + await user.click( + screen.getByRole("button", { name: "Cancel model lookup" }), + ); + await act(async () => release(result)); + expect(cancel).toHaveBeenCalled(); + expect(screen.getByRole("combobox", { name: "Model" })).toHaveValue("kept"); + await user.click(screen.getByRole("button", { name: "Retry models" })); + await waitFor(() => expect(run).toHaveBeenCalledTimes(2)); + draft = { ...draft, workspace: "/different/workspace" }; + view.rerender(renderPicker()); + await act(async () => release(result)); + expect( + screen.queryByRole("option", { name: /custom\/new/ }), + ).not.toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Browse models" })); + await waitFor(() => expect(run).toHaveBeenCalledTimes(3)); + await act(async () => release(result)); + expect( + await screen.findByRole("option", { name: /custom\/new/ }), + ).toBeVisible(); + } finally { + view.unmount(); + control.dispose(); + } +}); + +it("reopened Pi editor accepts a pasted qualified ID without doubling its provider", async () => { + const control = createAgentControl(controlFixture().host); + const user = userEvent.setup(); + let current = { + ...agentDraft(controlFixture().agent), + command: "/local/buzz-pi-acp", + args: "[]", + provider: "custom", + model: "old", + }; + function Editor() { + const [draft, setDraft] = useState(current); + current = draft; + return ( + setDraft((d) => ({ ...d, ...patch }))} + /> + ); + } + const view = render(); + try { + const input = screen.getByRole("combobox", { name: "Model" }); + await user.clear(input); + await user.type(input, "custom/namespace/model.v1"); + await user.tab(); + expect(current.model).toBe("namespace/model.v1"); + await user.clear(input); + await user.type(input, "namespace/other.v2"); + await user.tab(); + expect(current.model).toBe("namespace/other.v2"); + } finally { + view.unmount(); + control.dispose(); + } +}); + +it("Pi warns about incomplete or unlisted selections and preserves literal Advanced IDs", async () => { + const f = controlFixture(); + f.host.models = { + begin: async () => 1, + cancel: async () => {}, + run: async () => ({ + host: "", + models: [{ id: "custom/listed", name: "custom/listed" }], + modelOverridden: false, + disconnected: false, + }), + }; + const control = createAgentControl(f.host); + const user = userEvent.setup(); + let current = { + ...agentDraft(f.agent), + command: "/local/buzz-pi-acp", + provider: "custom", + model: "", + args: "[]", + }; + function Editor() { + const [draft, setDraft] = useState(current); + current = draft; + return ( + setDraft((d) => ({ ...d, ...patch }))} + /> + ); + } + const view = render(); + try { + expect( + screen.getByText(/Choose a model for this provider before starting/), + ).toBeVisible(); + await user.click(screen.getByRole("button", { name: "Model" })); + const literal = screen.getByLabelText("Model ID (custom or blank)"); + await user.type(literal, "custom/"); + expect(literal).toHaveValue("custom/"); + await user.type(literal, "real-model"); + await user.tab(); + expect(current.model).toBe("custom/real-model"); + await user.click(screen.getByRole("button", { name: "Browse models" })); + await screen.findByText(/This model ID is not in Pi’s available catalog/); + expect(current.model).toBe("custom/real-model"); + await user.click( + await screen.findByRole("option", { name: /custom\/listed/ }), + ); + expect(current.model).toBe("listed"); + expect( + screen.queryByText(/This model ID is not in Pi’s available catalog/), + ).not.toBeInTheDocument(); + } finally { + view.unmount(); + control.dispose(); + } +}); + +it("Pi clears discovered providers when catalog context changes or the picker unmounts", async () => { + const f = controlFixture(); + const run = vi.fn(async () => ({ + host: "", + models: [{ id: "extension/exact", name: "extension/exact" }], + modelOverridden: false, + disconnected: false, + })); + f.host.models = { begin: async () => 1, cancel: async () => {}, run }; + const control = createAgentControl(f.host), + providers = vi.fn(); + const user = userEvent.setup(); + let draft = { + ...agentDraft(f.agent), + command: "/local/buzz-pi-acp", + provider: "", + model: "", + args: "[]", + }; + const picker = () => ( + {}} + /> + ); + const view = render(picker()); + try { + for (const patch of [ + { workspace: "/new/workspace" }, + { environment: { PI_CODING_AGENT_DIR: "/new/config" } }, + { command: "buzz-agent" }, + ]) { + await user.click(screen.getByRole("button", { name: "Browse models" })); + await waitFor(() => + expect(providers).toHaveBeenLastCalledWith(["extension"]), + ); + await screen.findByRole("option", { name: /extension\/exact/ }); + await user.keyboard("{Escape}"); + draft = { ...draft, ...patch }; + view.rerender(picker()); + await waitFor(() => expect(providers).toHaveBeenLastCalledWith([])); + } + view.unmount(); + expect(providers).toHaveBeenLastCalledWith([]); + } finally { + view.unmount(); + control.dispose(); + } +}); diff --git a/src/bundled/agents/AgentModelPicker.tsx b/src/bundled/agents/AgentModelPicker.tsx index 512133965..a08ed2b9c 100644 --- a/src/bundled/agents/AgentModelPicker.tsx +++ b/src/bundled/agents/AgentModelPicker.tsx @@ -2,7 +2,7 @@ import { Accordion } from "../../shared/design-system/ui/Accordion"; import { Field } from "../../shared/design-system/ui/Field"; import { Input } from "../../shared/design-system/ui/Input"; import { Combobox } from "../../shared/design-system/ui/Combobox"; -import { useEffect, useRef, useState, useId } from "react"; +import { useEffect, useId, useRef, useState } from "react"; import type { AgentControl, ControlSnapshot, @@ -17,10 +17,12 @@ export function AgentModelPicker({ savedRevision, control, defaults, + onPiProviders, onChange, disabled = false, }: { disabled?: boolean; + onPiProviders?(providers: string[]): void; id?: string | undefined; savedRevision?: number | undefined; draft: AgentDraft; @@ -30,6 +32,8 @@ export function AgentModelPicker({ }) { const statusId = useId(); const goose = isGoose(draft.command); + const pi = draft.command.split("/").at(-1) === "buzz-pi-acp"; + const external = goose || pi; const host = draft.databricks?.host ?? defaults?.host ?? ""; const filter = draft.databricks?.filter ?? defaults?.filter ?? ""; const [catalog, setCatalog] = useState<{ @@ -52,11 +56,12 @@ export function AgentModelPicker({ savedRevision, draft.revision, draft.command, - draft.provider, + pi ? null : draft.provider, draft.args, + draft.workspace, draft.environment, - host, - filter, + pi ? null : host, + pi ? null : filter, ]); const currentKey = useRef(key); currentKey.current = key; @@ -69,14 +74,23 @@ export function AgentModelPicker({ setQuery(null); setOpen(false); attempted.current = null; + onPiProviders?.([]); return () => { pending.current?.abort(); pending.current = null; + onPiProviders?.([]); }; - }, [key]); + }, [key, onPiProviders]); + // Provider is only a filter for Pi's catalog, but pending search text belongs + // to the provider the person was editing. + // biome-ignore lint/correctness/useExhaustiveDependencies: provider changes retire its pending search text without invalidating Pi’s catalog. + useEffect(() => { + setQuery(null); + highlighted.current = null; + }, [draft.provider]); const run = async (action: "connect" | "refresh" | "disconnect") => { if (!control.models || pending.current) return; - if (!goose && !host.trim()) { + if (!external && !host.trim()) { setStatus( "Set your Databricks workspace under Advanced → Model to browse models.", ); @@ -89,8 +103,8 @@ export function AgentModelPicker({ setCatalog(null); setStatus( action === "connect" - ? goose - ? "Loading Goose models…" + ? external + ? `Loading ${pi ? "Pi" : "Goose"} models…` : "Loading models… sign in through your browser if asked." : action === "refresh" ? "Loading models…" @@ -102,21 +116,25 @@ export function AgentModelPicker({ id, expectedRevision: id ? draft.revision : undefined, edit: action === "disconnect" ? undefined : agentEdit(draft, true), - host: goose ? "" : host, - filter: goose ? "" : filter, + host: external ? "" : host, + filter: external ? "" : filter, action, }, abort.signal, ); if (abort.signal.aborted || currentKey.current !== key) return; setCatalog({ key, data }); + if (pi) + onPiProviders?.([ + ...new Set(data.models.map((m) => m.id.split("/")[0] ?? "")), + ]); setStatus( data.disconnected ? "Disconnected from this workspace in Foundation." : data.models.length ? "" - : goose - ? "No Goose models found. Check Goose configuration or enter a custom ID." + : external + ? `No ${pi ? "Pi" : "Goose"} models found. Check local configuration or enter a custom ID.` : "No models found. Enter a custom ID or check the workspace/filter under Advanced → Model.", ); } catch (error) { @@ -130,9 +148,32 @@ export function AgentModelPicker({ } }; const fresh = catalog?.key === key ? catalog.data : null; - const entries = fresh?.models ?? []; + const entries = (fresh?.models ?? []).filter( + (m) => !pi || !draft.provider || m.id.startsWith(`${draft.provider}/`), + ); + const selectedId = + pi && draft.provider && draft.model + ? `${draft.provider}/${draft.model}` + : draft.model; + const chooseModel = (value: string) => { + if (pi && value.includes("/") && entries.some((m) => m.id === value)) { + const split = value.indexOf("/"); + onChange({ + provider: value.slice(0, split), + model: value.slice(split + 1), + }); + } else { + const prefix = `${draft.provider}/`; + onChange({ + model: + pi && draft.provider && value.startsWith(prefix) + ? value.slice(prefix.length) + : value, + }); + } + }; const selected = - entries.find((model) => model.id === draft.model) ?? + entries.find((model) => model.id === selectedId) ?? (draft.model ? { id: draft.model, name: draft.model } : null); const items = [...entries]; if (selected && !entries.some((model) => model.id === selected.id)) @@ -148,7 +189,7 @@ export function AgentModelPicker({ const match = entries.find( (item) => item.id === query || item.name === query, ); - onChange({ model: match?.id ?? query }); + chooseModel(match?.id ?? query); setQuery(null); }; return ( @@ -198,7 +239,7 @@ export function AgentModelPicker({ itemToStringLabel={(model) => model.name} isItemEqualToValue={(a, b) => a.id === b.id} onValueChange={(model) => { - if (model) onChange({ model: model.id }); + if (model) chooseModel(model.id); setQuery(null); }} > @@ -266,7 +307,7 @@ export function AgentModelPicker({ setStatus("Cancelled. Retry when ready."); }} > - {goose ? "Cancel model lookup" : "Cancel sign-in"} + {external ? "Cancel model lookup" : "Cancel sign-in"} ) : ( status && @@ -276,6 +317,28 @@ export function AgentModelPicker({ ) )} + {pi && draft.provider && !draft.model && ( +

+ Choose a model for this provider before starting, or clear Provider + to use Pi defaults. +

+ )} + {pi && fresh && entries.length === 0 && draft.provider && ( +

+ No Pi models available for this provider. Check Pi sign-in or + extension configuration, or enter a custom ID. +

+ )} + {pi && + fresh && + draft.model && + !entries.some((model) => model.id === selectedId) && ( +

+ This model ID is not in Pi’s available catalog. Select a listed + model or confirm the exact custom ID before starting; Pi may + accept an invalid ID until the first message. +

+ )} {fresh?.modelOverridden && (

{goose ? "A GOOSE_MODEL" : "A saved BUZZ_AGENT_MODEL"} environment @@ -320,7 +383,22 @@ export function AgentModelPicker({ } /> - {supported && !goose && ( + {pi && ( +

+ This field uses the exact model ID, including any + namespace slashes, without adding the provider. Its text + is saved literally. +

+ )} + {supported && pi && ( + + )} + {supported && !external && ( <> ): void; }) { + const [piProviders, setPiProviders] = useState([]); + const pi = draft.command.split("/").at(-1) === "buzz-pi-acp"; const goose = isGoose(draft.command); const providerOverride = draft.environment.GOOSE_PROVIDER; const provider = providerOverride ?? draft.provider; @@ -66,6 +69,7 @@ export function AgentSettingsFields({ disabled={disabled} draft={draft} options={state.data?.harnessOptions ?? []} + piProviders={piProviders} onChange={onChange} /> {goose && !gooseCanBrowse ? ( @@ -86,6 +90,7 @@ export function AgentSettingsFields({
) : ( )} + {pi && ( +

+ Browse loads available models and providers from your local Pi + configuration, including extensions. Configure sign-in in Pi + first. Save keeps changes for the next Start or Restart. +

+ )}
@@ -134,9 +146,10 @@ export function AgentSettingsFields({ onChange={(environment) => onChange({ environment })} />

- Environment overrides take precedence over provider and - model selections. Arguments are passed literally, not - through a shell. + {pi + ? 'Pi needs both Provider and Model to override its defaults. Advanced Pi options follow --; for example: ["--", "--extension", "/absolute/path/to/extension.ts"]. PI_CODING_AGENT_DIR can select a local Pi configuration directory.' + : "Environment overrides take precedence over provider and model selections."}{" "} + Arguments are passed literally, not through a shell.

), From 3067b8fc7b8df19b0f36286d6991af04828d8dd7 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Thu, 24 Sep 2026 11:31:59 -0400 Subject: [PATCH 2/5] Preserve Goose custom path handling after Pi integration Signed-off-by: Salman Mohammed --- .../agents/AgentHarnessEditor.test.tsx | 115 ++++++++++-------- src/bundled/agents/AgentHarnessEditor.tsx | 7 +- 2 files changed, 68 insertions(+), 54 deletions(-) diff --git a/src/bundled/agents/AgentHarnessEditor.test.tsx b/src/bundled/agents/AgentHarnessEditor.test.tsx index 1253e9922..44a64cda0 100644 --- a/src/bundled/agents/AgentHarnessEditor.test.tsx +++ b/src/bundled/agents/AgentHarnessEditor.test.tsx @@ -67,62 +67,73 @@ it("keeps custom mode separate from saved values and supports an unset provider" ); }); -it("preserves Goose settings while editing a custom executable path", async () => { - const f = controlFixture(); - const user = userEvent.setup(); - function Example() { - const [draft, setDraft] = useState({ - ...agentDraft(f.agent), - command: "/usr/local/bin/goose", +it.each(["/opt/homebrew/bin/goose", "C:\\tools\\goose"])( + "preserves Goose settings while editing custom executable %s", + async (path) => { + const f = controlFixture(); + const user = userEvent.setup(); + function Example() { + const [draft, setDraft] = useState({ + ...agentDraft(f.agent), + command: "/usr/local/bin/goose", + args: '["acp"]', + provider: "openrouter", + model: "m", + }); + return ( + <> + + setDraft((current) => ({ ...current, ...patch })) + } + /> + {JSON.stringify(draft)} + + ); + } + render(); + await user.click(screen.getByRole("combobox", { name: "Harness" })); + await user.click( + await screen.findByRole("option", { + name: "Custom executable / current value", + }), + ); + const executable = screen.getByRole("textbox", { name: "Executable" }); + await user.clear(executable); + await user.type(executable, path); + expect( + JSON.parse(screen.getByRole("status").textContent ?? ""), + ).toMatchObject({ + command: path, args: '["acp"]', provider: "openrouter", model: "m", }); - return ( - <> - - setDraft((current) => ({ ...current, ...patch })) - } - /> - {JSON.stringify(draft)} - - ); - } - render(); - await user.click(screen.getByRole("combobox", { name: "Harness" })); - await user.click( - await screen.findByRole("option", { - name: "Custom executable / current value", - }), - ); - const executable = screen.getByRole("textbox", { name: "Executable" }); - await user.clear(executable); - await user.type(executable, "/opt/homebrew/bin/goose"); - expect( - JSON.parse(screen.getByRole("status").textContent ?? ""), - ).toMatchObject({ - command: "/opt/homebrew/bin/goose", - args: '["acp"]', - provider: "openrouter", - model: "m", - }); -}); + await user.click(screen.getByRole("combobox", { name: "LLM Provider" })); + await user.click(await screen.findByRole("option", { name: "Not set" })); + expect( + JSON.parse(screen.getByRole("status").textContent ?? ""), + ).toMatchObject({ + provider: "", + model: "", + }); + }, +); it("switching Pi, Goose and Buzz resets incompatible selections and uses each harness arguments", async () => { let current = { diff --git a/src/bundled/agents/AgentHarnessEditor.tsx b/src/bundled/agents/AgentHarnessEditor.tsx index ce262d553..0f2ca35f1 100644 --- a/src/bundled/agents/AgentHarnessEditor.tsx +++ b/src/bundled/agents/AgentHarnessEditor.tsx @@ -19,10 +19,13 @@ export function AgentHarnessEditor({ disabled?: boolean; onChange(patch: Partial): void; }) { - const executable = draft.command.split("/").at(-1); + const executable = draft.command.replaceAll("\\", "/").split("/").at(-1); const harness = options.find((option) => option.command === draft.command) ?? - options.find((option) => option.command.split("/").at(-1) === executable); + options.find( + (option) => + option.command.replaceAll("\\", "/").split("/").at(-1) === executable, + ); const external = harness?.label === "Goose" || harness?.label === "Pi"; return (
From 6cafab476f68fc40c2e6f4db18c5d37f5557a1b6 Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Thu, 24 Sep 2026 14:17:50 -0400 Subject: [PATCH 3/5] Keep custom Buzz executable providers editable Signed-off-by: Salman Mohammed --- src/bundled/agents/AgentHarnessEditor.test.tsx | 4 ++-- src/bundled/agents/AgentHarnessEditor.tsx | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/bundled/agents/AgentHarnessEditor.test.tsx b/src/bundled/agents/AgentHarnessEditor.test.tsx index 44a64cda0..55a434b30 100644 --- a/src/bundled/agents/AgentHarnessEditor.test.tsx +++ b/src/bundled/agents/AgentHarnessEditor.test.tsx @@ -55,7 +55,7 @@ it("keeps custom mode separate from saved values and supports an unset provider" await user.clear(screen.getByRole("textbox", { name: "Executable" })); await user.type( screen.getByRole("textbox", { name: "Executable" }), - "/custom/agent", + "/custom/buzz-agent", ); expect(screen.getByRole("textbox", { name: "Custom provider" })).toHaveValue( "provider", @@ -63,7 +63,7 @@ it("keeps custom mode separate from saved values and supports an unset provider" await user.click(screen.getByRole("combobox", { name: "Provider" })); await user.click(await screen.findByRole("option", { name: "Not set" })); expect(screen.getByRole("status")).toHaveTextContent( - '{"command":"/custom/agent","provider":""}', + '{"command":"/custom/buzz-agent","provider":""}', ); }); diff --git a/src/bundled/agents/AgentHarnessEditor.tsx b/src/bundled/agents/AgentHarnessEditor.tsx index 0f2ca35f1..1e216a639 100644 --- a/src/bundled/agents/AgentHarnessEditor.tsx +++ b/src/bundled/agents/AgentHarnessEditor.tsx @@ -22,10 +22,13 @@ export function AgentHarnessEditor({ const executable = draft.command.replaceAll("\\", "/").split("/").at(-1); const harness = options.find((option) => option.command === draft.command) ?? - options.find( - (option) => - option.command.replaceAll("\\", "/").split("/").at(-1) === executable, - ); + (executable === "goose" || executable === "buzz-pi-acp" + ? options.find( + (option) => + option.command.replaceAll("\\", "/").split("/").at(-1) === + executable, + ) + : undefined); const external = harness?.label === "Goose" || harness?.label === "Pi"; return (
From a82ecb069db39de90abd45d60a494bf2dd64360f Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Thu, 24 Sep 2026 15:20:53 -0400 Subject: [PATCH 4/5] Show model lookup progress and bound the results list Signed-off-by: Salman Mohammed --- src/bundled/agents/AgentModelPicker.test.tsx | 15 ++++++++++- src/bundled/agents/AgentModelPicker.tsx | 28 +++++++++++++++----- tests/browser/agent-models.spec.mjs | 23 ++++++++++++++++ tests/fixtures/agent-control.tsx | 6 +++++ 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/src/bundled/agents/AgentModelPicker.test.tsx b/src/bundled/agents/AgentModelPicker.test.tsx index b35f4169e..4e25ab981 100644 --- a/src/bundled/agents/AgentModelPicker.test.tsx +++ b/src/bundled/agents/AgentModelPicker.test.tsx @@ -181,7 +181,7 @@ it("Pi discovers extension providers before start and selects the exact provider command: "/local/buzz-pi-acp", args: "[]", provider: "", - model: "", + model: "saved-custom", }; function Editor() { const [draft, setDraft] = useState(current); @@ -203,6 +203,13 @@ it("Pi discovers extension providers before start and selects the exact provider expect( screen.getByRole("button", { name: "Cancel model lookup", hidden: true }), ).toBeVisible(); + expect( + await screen.findByRole("status", { name: "Model lookup" }), + ).toHaveTextContent("Loading Pi models…"); + expect(screen.getByRole("combobox", { name: "Model" })).toHaveAttribute( + "aria-busy", + "true", + ); await act(async () => release({ host: "", @@ -221,6 +228,12 @@ it("Pi discovers extension providers before start and selects the exact provider name: /extension\/namespace\/model.v1/, }), ); + expect( + screen.queryByRole("status", { name: "Model lookup" }), + ).not.toBeInTheDocument(); + expect(screen.getByRole("combobox", { name: "Model" })).not.toHaveAttribute( + "aria-busy", + ); expect(current.provider).toBe("extension"); expect(current.model).toBe("namespace/model.v1"); expect(providers).toHaveBeenCalledWith(["extension"]); diff --git a/src/bundled/agents/AgentModelPicker.tsx b/src/bundled/agents/AgentModelPicker.tsx index a08ed2b9c..02308b1ff 100644 --- a/src/bundled/agents/AgentModelPicker.tsx +++ b/src/bundled/agents/AgentModelPicker.tsx @@ -8,6 +8,7 @@ import type { ControlSnapshot, } from "../../features/agents/control"; import type { ModelCatalog } from "../../features/agents/models"; +import { CircleNotchIcon } from "../../shared/design-system/icons"; import { Button } from "../../shared/design-system/ui/Button"; import { agentEdit, isGoose, type AgentDraft } from "./agent-edit"; @@ -268,13 +269,28 @@ export function AgentModelPicker({ }} /> - + {busy && ( +
+
+ )} + {(model: ModelCatalog["models"][number]) => ( window.agentModelsFixture.calls)).toEqual( [], ); + // Only this geometry check opts into a catalog larger than the popup. + await page.evaluate(() => window.agentModelsFixture.mode("many")); await browse.click(); await expect( page.getByRole("option", { name: /Friendly Model/ }), ).toBeVisible(); + const list = page.getByRole("listbox"); + await expect(page.getByRole("option")).toHaveCount(21); + const bounds = await list.evaluate((element) => ({ + height: element.clientHeight, + content: element.scrollHeight, + })); + expect(bounds.height).toBeLessThanOrEqual(320); + expect(bounds.content).toBeGreaterThan(bounds.height); + const lastModel = page.getByRole("option", { name: /Catalog Model 20/ }); + await lastModel.scrollIntoViewIfNeeded(); + await expect + .poll(() => list.evaluate((element) => element.scrollTop)) + .toBeGreaterThan(0); + await search.fill("Catalog Model 20"); + await expect(lastModel).toBeVisible(); + await expect( + page.getByRole("option", { name: /Friendly Model/ }), + ).toHaveCount(0); + await search.press("Escape"); + await browse.click(); + await page.evaluate(() => window.agentModelsFixture.mode("success")); await expect(model).toHaveValue("custom.keep"); await page.getByRole("option", { name: /Friendly Model/ }).click(); await expect(model).toHaveValue("catalog.schema.real-model"); diff --git a/tests/fixtures/agent-control.tsx b/tests/fixtures/agent-control.tsx index 3f66ced9b..2e13394cb 100644 --- a/tests/fixtures/agent-control.tsx +++ b/tests/fixtures/agent-control.tsx @@ -43,6 +43,12 @@ fixture.host.models = { : [ { id: "catalog.schema.real-model", name: "Friendly Model" }, { id: "endpoint-two", name: "Other Model" }, + ...(modelMode === "many" + ? Array.from({ length: 18 }, (_, index) => ({ + id: `endpoint-${index + 3}`, + name: `Catalog Model ${index + 3}`, + })) + : []), ], modelOverridden: false, disconnected: request.action === "disconnect", From 834a82be7b9d09def8d391c666c385e8eed5bc4e Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Thu, 24 Sep 2026 15:20:55 -0400 Subject: [PATCH 5/5] Validate Pi catalog selections against launch constraints Signed-off-by: Salman Mohammed --- crates/agent-controller/src/pi.rs | 37 +++++++++++++------- crates/agent-controller/src/runtime/tests.rs | 19 ++++++++++ src-tauri/src/pi_models.rs | 8 ++--- src-tauri/src/pi_models/tests.rs | 24 +++++++++++++ 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/crates/agent-controller/src/pi.rs b/crates/agent-controller/src/pi.rs index 390e285db..c27f339e1 100644 --- a/crates/agent-controller/src/pi.rs +++ b/crates/agent-controller/src/pi.rs @@ -142,18 +142,7 @@ impl PiContext { } pub(crate) fn adapter_args(&self, harness: &HarnessEdit) -> Result> { - if !harness.provider.is_empty() && harness.model.is_empty() { - return Err("Choose a Pi model for the selected provider, or clear both fields to use Pi defaults".into()); - } - for value in [&harness.provider, &harness.model] { - if value.len() > 512 - || value.starts_with('-') - || value.contains([',', '\0']) - || value.chars().any(char::is_control) - { - return Err("Invalid Pi provider or model ID".into()); - } - } + validate_selection(&harness.provider, &harness.model)?; let mut args = vec!["--".into()]; args.extend(self.args.clone()); if !harness.provider.is_empty() { @@ -165,3 +154,27 @@ impl PiContext { Ok(args) } } + +/// Selection constraints shared by catalog discovery and ACP launch. +/// Empty selectors retain Pi defaults; model IDs may contain namespace slashes. +pub fn validate_selection(provider: &str, model: &str) -> Result<()> { + if !provider.is_empty() && model.is_empty() { + return Err( + "Choose a Pi model for the selected provider, or clear both fields to use Pi defaults" + .into(), + ); + } + if provider.contains('/') { + return Err("Invalid Pi provider or model ID".into()); + } + for (value, limit) in [(provider, 128), (model, 512)] { + if value.len() > limit + || value.starts_with('-') + || value.contains(',') + || value.chars().any(char::is_control) + { + return Err("Invalid Pi provider or model ID".into()); + } + } + Ok(()) +} diff --git a/crates/agent-controller/src/runtime/tests.rs b/crates/agent-controller/src/runtime/tests.rs index f94d47714..358e07743 100644 --- a/crates/agent-controller/src/runtime/tests.rs +++ b/crates/agent-controller/src/runtime/tests.rs @@ -809,6 +809,25 @@ fn pi_selection_and_extensions_survive_save_reopen_and_reach_adapter() { controller.store.agents().unwrap()[0].environment["PI_CODING_AGENT_DIR"], a.environment["PI_CODING_AGENT_DIR"] ); + for (provider, model) in [ + ("custom".to_owned(), "a,b".to_owned()), + ("custom".to_owned(), "-model".to_owned()), + ("custom,other".to_owned(), "model".to_owned()), + ("-provider".to_owned(), "model".to_owned()), + ("bad/provider".to_owned(), "model".to_owned()), + ("p".repeat(129), "model".to_owned()), + ("custom".to_owned(), "m".repeat(513)), + ] { + let mut invalid = saved.clone(); + invalid.harness.provider = provider; + invalid.harness.model = model; + assert!(controller + .bundle + .as_ref() + .unwrap() + .command(&invalid, &key) + .is_err()); + } let mut configured = saved.clone(); configured.harness.model.clear(); assert!(controller diff --git a/src-tauri/src/pi_models.rs b/src-tauri/src/pi_models.rs index fab430563..4c479fdbf 100644 --- a/src-tauri/src/pi_models.rs +++ b/src-tauri/src/pi_models.rs @@ -94,13 +94,11 @@ fn parse_response(value: &Value) -> Result, String> { for model in models { let provider = model["provider"].as_str().ok_or(FAILURE)?; let id = model["id"].as_str().ok_or(FAILURE)?; - if [provider, id] - .iter() - .any(|s| s.is_empty() || s.len() > 512 || s.chars().any(char::is_control)) - || provider.contains('/') - { + if provider.is_empty() || id.is_empty() { return Err("Pi returned an invalid model ID".into()); } + buzz_agent_controller::pi::validate_selection(provider, id) + .map_err(|_| "Pi returned an invalid model ID")?; result.push(format!("{provider}/{id}")); } result.sort(); diff --git a/src-tauri/src/pi_models/tests.rs b/src-tauri/src/pi_models/tests.rs index 28c6e51ea..0456acba0 100644 --- a/src-tauri/src/pi_models/tests.rs +++ b/src-tauri/src/pi_models/tests.rs @@ -14,6 +14,30 @@ fn catalog_keeps_exact_ids_and_provider_boundaries_and_redacts_errors() { assert!(!error.contains("secret")); } } +#[test] +fn catalog_rejects_selections_that_save_or_launch_cannot_accept() { + for (provider, id) in [ + ("custom".to_owned(), "a,b".to_owned()), + ("custom".to_owned(), "-model".to_owned()), + ("custom,other".to_owned(), "model".to_owned()), + ("-provider".to_owned(), "model".to_owned()), + ("p".repeat(129), "model".to_owned()), + ("custom".to_owned(), "m".repeat(513)), + ] { + let response = json!({"type":"response","command":"get_available_models","success":true, + "data":{"models":[{"provider":provider,"id":id}]}}); + assert!(parse_response(&response).is_err(), "{provider}/{id}"); + } + let provider = "p".repeat(128); + let id = format!("namespace/{}", "m".repeat(502)); + let response = json!({"type":"response","command":"get_available_models","success":true, + "data":{"models":[{"provider":provider,"id":id}]}}); + assert_eq!( + parse_response(&response).unwrap(), + [format!("{provider}/{id}")] + ); +} + #[cfg(unix)] fn fixture(script: &str) -> (tempfile::TempDir, PiContext) { use std::os::unix::fs::PermissionsExt;