diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index a4061aa3a4e..74da36d65bc 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -125,7 +125,10 @@ const overrides = new Map([ // Windows-CI portability: replaced POSIX true/false probes with current_exe() // stand-in + present_binary_str()/static_commands() helpers (+29 lines). // Tests now pass on windows-latest CI shard without POSIX shell utilities. - ["src-tauri/src/managed_agents/readiness.rs", 1403], + // databricks-v1-to-v2-migration: databricks-v2 hyphen-alias added to all + // host/credential match arms + 30+ readiness tests for provider aliases, + // missing-host, and DATABRICKS_MODEL fallback. Load-bearing correctness fix. + ["src-tauri/src/managed_agents/readiness.rs", 1546], // applyWorkspace reposDir parameter plus the validateReposDir binding, // threaded through Tauri invokes for configurable repos_dir, plus the // harness-persona-sync `harnessOverride` create-input bit — load-bearing @@ -185,7 +188,11 @@ const overrides = new Map([ // the pre-identity data migrations; still queued to split further. // unified-agent-model 1A.1: materialize_agent_runtimes split to // migration/materialize.rs, ratcheting 1310 -> 1297. - ["src-tauri/src/migration.rs", 1297], + // databricks-v1-to-v2-migration: reconcile_databricks_v1_to_v2 migration + // + inner fn with baked-env gate + 26 tests. Load-bearing correctness fix. + // am review fix: also clear stale V1 model field on provider rewrite + + // new model-clear test. Load-bearing chimera fix. + ["src-tauri/src/migration.rs", 1402], // onMarkRead + isUnread prop threading (mirrors the onMarkUnread prop // already here) for the single-toggle mark-read/unread menu item — a small // overage from load-bearing per-message plumbing, not generic debt growth. diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index ae10d92b137..17938e0b903 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -638,12 +638,14 @@ fn is_databricks_provider(provider: Option<&str>) -> bool { .map(str::trim) .map(str::to_ascii_lowercase) .as_deref(), - Some("databricks" | "databricks_v2") + Some("databricks" | "databricks_v2" | "databricks-v2") ) } fn databricks_agent_provider(provider: &str) -> buzz_agent_pkg::config::Provider { - if provider.trim().eq_ignore_ascii_case("databricks_v2") { + if provider.trim().eq_ignore_ascii_case("databricks_v2") + || provider.trim().eq_ignore_ascii_case("databricks-v2") + { buzz_agent_pkg::config::Provider::DatabricksV2 } else { buzz_agent_pkg::config::Provider::Databricks diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index aff4bf17ae3..7673b1f267e 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -276,12 +276,29 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { // Model is required — maps to BUZZ_AGENT_MODEL in the effective env. // Same empty-string treatment as provider. - let model = effective + // Also accept provider-specific model fallback keys, matching buzz-agent's + // own config.rs `from_env()` resolution order (e.g. DATABRICKS_MODEL for + // databricks/databricks_v2, ANTHROPIC_MODEL for anthropic, etc.). The + // baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL, + // so without this fallback agents baked from releases appear "not ready". + let provider_model_key = match provider { + Some("databricks") | Some("databricks_v2") | Some("databricks-v2") => { + Some("DATABRICKS_MODEL") + } + Some("anthropic") => Some("ANTHROPIC_MODEL"), + Some("openai") | Some("openai-compat") => Some("OPENAI_COMPAT_MODEL"), + _ => None, + }; + let model_present = effective .env .get("BUZZ_AGENT_MODEL") .filter(|v| !v.is_empty()) - .map(String::as_str); - if model.is_none() { + .is_some() + || provider_model_key + .and_then(|k| effective.env.get(k)) + .filter(|v| !v.is_empty()) + .is_some(); + if !model_present { missing.push(Requirement::NormalizedField { field: "model".to_string(), }); @@ -304,7 +321,7 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { key: "OPENAI_COMPAT_API_KEY".to_string(), }); } - Some("databricks") | Some("databricks_v2") + Some("databricks") | Some("databricks_v2") | Some("databricks-v2") // DATABRICKS_HOST is hard-required; DATABRICKS_TOKEN is optional // (OAuth PKCE is the normal path — see buzz-agent/src/config.rs:143). if env_key_missing("DATABRICKS_HOST") => { @@ -412,7 +429,7 @@ fn goose_requirements( key: "OPENAI_COMPAT_API_KEY".to_string(), }); } - Some("databricks") | Some("databricks_v2") + Some("databricks") | Some("databricks_v2") | Some("databricks-v2") if env_key_missing("DATABRICKS_HOST") && !file_key_present("DATABRICKS_HOST") => { missing.push(Requirement::EnvKey { @@ -1199,6 +1216,141 @@ mod tests { Some("claude-opus-4-5") ); } + + // ── provider-specific model fallback tests ──────────────────────────── + + #[test] + fn buzz_agent_databricks_v2_with_databricks_model_but_no_buzz_agent_model_is_ready() { + // The baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL. + // An agent with only DATABRICKS_MODEL must pass the readiness gate. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks_v2"), + ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "DATABRICKS_MODEL must satisfy the model requirement for databricks_v2" + ); + } + + #[test] + fn buzz_agent_databricks_v2_hyphen_alias_with_databricks_model_is_ready() { + // buzz-agent accepts both "databricks_v2" and "databricks-v2". The + // readiness gate must recognize the hyphen alias and accept DATABRICKS_MODEL. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks-v2"), + ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "databricks-v2 alias with DATABRICKS_MODEL must be Ready" + ); + } + + #[test] + fn buzz_agent_databricks_hyphen_alias_missing_host_returns_not_ready() { + // The hyphen alias "databricks-v2" requires DATABRICKS_HOST just like + // the underscore variants. Without it the agent cannot reach the endpoint. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks-v2"), + ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), + // DATABRICKS_HOST intentionally absent + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "databricks-v2 without DATABRICKS_HOST must be NotReady" + ); + let reqs = result.requirements(); + assert!( + reqs.iter() + .any(|r| matches!(r, Requirement::EnvKey { key } if key == "DATABRICKS_HOST")), + "missing requirements must include DATABRICKS_HOST; got {reqs:?}" + ); + } + + #[test] + fn buzz_agent_databricks_v1_with_databricks_model_but_no_buzz_agent_model_is_ready() { + // V1 (Model Serving) also resolves DATABRICKS_MODEL — same fallback applies. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks"), + ("DATABRICKS_MODEL", "dbrx-instruct"), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "DATABRICKS_MODEL must satisfy the model requirement for databricks (V1)" + ); + } + + #[test] + fn buzz_agent_anthropic_with_anthropic_model_but_no_buzz_agent_model_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "anthropic"), + ("ANTHROPIC_MODEL", "claude-opus-4-5"), + ("ANTHROPIC_API_KEY", "sk-test"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "ANTHROPIC_MODEL must satisfy the model requirement for anthropic" + ); + } + + #[test] + fn buzz_agent_openai_with_openai_compat_model_but_no_buzz_agent_model_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openai"), + ("OPENAI_COMPAT_MODEL", "gpt-4o"), + ("OPENAI_COMPAT_API_KEY", "sk-test"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "OPENAI_COMPAT_MODEL must satisfy the model requirement for openai" + ); + } + + #[test] + fn buzz_agent_empty_provider_model_fallback_key_is_not_ready() { + // An empty DATABRICKS_MODEL with no BUZZ_AGENT_MODEL must still be NotReady. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks_v2"), + ("DATABRICKS_MODEL", ""), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "empty DATABRICKS_MODEL with no BUZZ_AGENT_MODEL must be NotReady" + ); + assert!(result + .requirements() + .contains(&Requirement::NormalizedField { + field: "model".to_string() + })); + } } // ── goose file-config–aware requirement tests ───────────────────────────── diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index efbc1e1fb87..fb3b033737a 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -165,6 +165,7 @@ pub fn run_boot_migrations(app: &tauri::AppHandle) { eprintln!("buzz-desktop: sync-team-personas: {e}"); } reconcile_provider_mcp_commands(app); + reconcile_databricks_v1_to_v2(app); materialize_agent_runtimes(app); } @@ -1234,6 +1235,114 @@ pub fn reconcile_provider_mcp_commands(app: &tauri::AppHandle) { } } +fn reconcile_databricks_v1_to_v2_in_file(path: &Path, rewrite_v1_provider: bool) { + use crate::managed_agents::is_derived_provider_model_key; + patch_json_records(path, |obj| { + let mut changed = false; + + // Only rewrite the structured provider field when the baked build env + // marks this as a Block build (BUZZ_AGENT_PROVIDER == "databricks_v2"). + // OSS users may intentionally select V1 (Model Serving), so we must not + // silently migrate their provider to V2 (AI Gateway). + if rewrite_v1_provider && obj.get("provider").and_then(|v| v.as_str()) == Some("databricks") + { + let name = obj + .get("name") + .and_then(|v| v.as_str()) + .unwrap_or("?") + .to_string(); + eprintln!( + "buzz-desktop: databricks-v1-to-v2: {name:?}: provider \"databricks\" → \"databricks_v2\"", + ); + obj.insert( + "provider".to_string(), + serde_json::Value::String("databricks_v2".to_string()), + ); + // Also clear the model field — a V1 model name (e.g. "dbrx-instruct") + // on a V2 provider would shadow the baked DATABRICKS_MODEL at spawn time + // (BUZZ_AGENT_MODEL from runtime_metadata_env_vars takes priority in + // buzz-agent config.rs). Clearing it lets the baked V2 default win. + if obj.remove("model").is_some() { + eprintln!( + "buzz-desktop: databricks-v1-to-v2: {name:?}: cleared stale V1 model field", + ); + } + changed = true; + } + + // Strip derived provider/model keys from env_vars on ALL records, + // regardless of rewrite_v1_provider. These keys are re-derived from + // structured fields at spawn time; stale copies in env_vars silently + // override the structured fields (last-write-wins in Command::env) and + // can cause V1 routing even when the provider dropdown shows V2. + // + // The check is case-insensitive (matching the established helper) + // to cover any case-variant that may have been written historically. + if let Some(serde_json::Value::Object(env_vars)) = obj.get_mut("env_vars") { + let stale_keys: Vec = env_vars + .keys() + .filter(|k| is_derived_provider_model_key(k)) + .cloned() + .collect(); + for key in stale_keys { + env_vars.remove(key.as_str()); + eprintln!("buzz-desktop: databricks-v1-to-v2: removed stale env_vars[\"{key}\"]",); + changed = true; + } + } + + changed + }); +} + +/// Strip stale derived provider/model keys from `env_vars` in all +/// managed-agent records, and — on Block builds — also migrate any persisted +/// `provider: "databricks"` to `"databricks_v2"`. +/// +/// **Block builds** (where `baked_build_env()` contains +/// `BUZZ_AGENT_PROVIDER=databricks_v2`): the structured `provider` field is +/// rewritten V1→V2 because the baked release targets V2 exclusively. Records +/// that were saved before this migration would otherwise silently override the +/// baked value at spawn time (last-write-wins in `Command::env`). +/// +/// **OSS builds** (baked env empty): the `provider` field is left alone — +/// V1 (`databricks`) is a valid Model Serving choice for OSS users. +/// +/// In both cases, stale `BUZZ_AGENT_PROVIDER` / `BUZZ_AGENT_MODEL` / +/// `GOOSE_PROVIDER` / `GOOSE_MODEL` are stripped from `env_vars`. These keys +/// are always re-derived from structured fields at spawn time; persisted copies +/// silence UI edits and cause stale routing. +/// +/// Covers both the current app data dir and the canonical dev data dir +/// (for worktree instances) — same dual-dir pattern as +/// `reconcile_legacy_command_names` and `reconcile_provider_mcp_commands`. +pub fn reconcile_databricks_v1_to_v2(app: &tauri::AppHandle) { + use crate::managed_agents::baked_build_env; + // On Block builds, the baked env contains BUZZ_AGENT_PROVIDER=databricks_v2. + // Use that as a reliable signal that this is a Block build and the V1 + // provider should be migrated. OSS builds have an empty baked env, so + // rewrite_v1_provider is false and the structured provider is preserved. + let rewrite_v1_provider = baked_build_env() + .get("BUZZ_AGENT_PROVIDER") + .map(|v| v == "databricks_v2") + .unwrap_or(false); + let Ok(current_dir) = app.path().app_data_dir() else { + return; + }; + let mut dirs = vec![current_dir.clone()]; + if let Some(canonical) = canonical_dev_data_dir(¤t_dir) { + if canonical.exists() && canonical != current_dir { + dirs.push(canonical); + } + } + for dir in dirs { + let path = dir.join("agents/managed-agents.json"); + if path.exists() { + reconcile_databricks_v1_to_v2_in_file(&path, rewrite_v1_provider); + } + } +} + fn rename_provider_to_runtime_in_personas(path: &Path) { patch_json_records(path, |obj| { if obj.contains_key("runtime") { @@ -1279,6 +1388,10 @@ mod tests; #[path = "migration_command_tests.rs"] mod command_tests; +#[cfg(test)] +#[path = "migration_databricks_tests.rs"] +mod databricks_tests; + #[cfg(test)] #[path = "migration_team_dir_tests.rs"] mod team_dir_tests; diff --git a/desktop/src-tauri/src/migration_databricks_tests.rs b/desktop/src-tauri/src/migration_databricks_tests.rs new file mode 100644 index 00000000000..2e50fae17de --- /dev/null +++ b/desktop/src-tauri/src/migration_databricks_tests.rs @@ -0,0 +1,360 @@ +use super::test_support::*; +use super::*; + +// ── reconcile_databricks_v1_to_v2_in_file ──────────────────────────────── + +#[test] +fn reconcile_databricks_v1_to_v2_rewrites_v1_provider_on_block_build() { + // rewrite_v1_provider=true simulates a Block build (baked env has + // BUZZ_AGENT_PROVIDER=databricks_v2). The structured provider field + // must be migrated V1→V2 and the stale V1 model field must be cleared + // so the baked DATABRICKS_MODEL wins at spawn time instead of the V1 name. + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Brain", + "provider": "databricks", + "model": "dbrx-instruct" + }]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ true, + ); + + let records = read_agents_json(dir.path()); + assert_eq!( + records[0]["provider"], "databricks_v2", + "provider: \"databricks\" must be rewritten to \"databricks_v2\" on Block builds" + ); + // Stale V1 model must be cleared so the baked DATABRICKS_MODEL is not + // shadowed by BUZZ_AGENT_MODEL at spawn time (last-write-wins in Command::env). + assert!( + records[0].get("model").map_or(true, |v| v.is_null()), + "stale V1 model field must be cleared when provider is rewritten to V2" + ); +} + +#[test] +fn reconcile_databricks_v1_to_v2_preserves_v1_provider_on_oss_build() { + // rewrite_v1_provider=false simulates an OSS build (empty baked env). + // V1 ("databricks") is a valid Model Serving provider for OSS users; + // the structured provider field must NOT be rewritten. + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Brain", + "provider": "databricks", + "model": "dbrx-instruct", + "env_vars": { "BUZZ_AGENT_PROVIDER": "databricks" } + }]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ false, + ); + + let records = read_agents_json(dir.path()); + // Provider field preserved. + assert_eq!( + records[0]["provider"], "databricks", + "provider field must not be rewritten on OSS builds" + ); + assert_eq!(records[0]["model"], "dbrx-instruct"); + // Stale env var is still stripped even on OSS builds. + assert!( + records[0]["env_vars"].get("BUZZ_AGENT_PROVIDER").is_none(), + "BUZZ_AGENT_PROVIDER must be stripped even when provider rewrite is disabled" + ); +} + +#[test] +fn reconcile_databricks_v1_to_v2_clears_model_on_provider_rewrite() { + // When a V1 record is migrated to V2 on a Block build, the model field + // must be removed. A stale V1 model name (e.g. "dbrx-instruct") emitted + // via BUZZ_AGENT_MODEL at spawn time would shadow the baked DATABRICKS_MODEL + // (last-write-wins), sending the agent to a V1 model on V2 endpoints. + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([ + { "name": "A", "provider": "databricks", "model": "dbrx-instruct" }, + { "name": "B", "provider": "databricks", "model": "goose-claude-opus-4-8-wrong" }, + // V2 record with model — model must NOT be cleared. + { "name": "C", "provider": "databricks_v2", "model": "goose-claude-4-8-opus" } + ]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ true, + ); + + let records = read_agents_json(dir.path()); + // V1 records: provider migrated, model cleared. + assert_eq!(records[0]["provider"], "databricks_v2"); + assert!( + records[0].get("model").map_or(true, |v| v.is_null()), + "model must be cleared for V1→V2 migrated record A" + ); + assert_eq!(records[1]["provider"], "databricks_v2"); + assert!( + records[1].get("model").map_or(true, |v| v.is_null()), + "model must be cleared for V1→V2 migrated record B" + ); + // V2 record: model untouched. + assert_eq!(records[2]["provider"], "databricks_v2"); + assert_eq!( + records[2]["model"], "goose-claude-4-8-opus", + "model must not be cleared for already-V2 record C" + ); +} + +#[test] +fn reconcile_databricks_v1_to_v2_preserves_v2_provider() { + let dir = tempfile::tempdir().unwrap(); + let json = serde_json::json!([{ + "name": "Brain", + "provider": "databricks_v2", + "model": "goose-claude-4-6-sonnet" + }]); + write_agents_json(dir.path(), &json); + let path = dir.path().join("agents/managed-agents.json"); + let before = std::fs::read_to_string(&path).unwrap(); + + reconcile_databricks_v1_to_v2_in_file(&path, /*rewrite_v1_provider=*/ true); + + // File must be unchanged — no spurious re-write. + assert_eq!(before, std::fs::read_to_string(&path).unwrap()); +} + +#[test] +fn reconcile_databricks_v1_to_v2_strips_stale_buzz_agent_provider_from_env_vars() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Brain", + "provider": "databricks_v2", + "model": "goose-claude-4-6-sonnet", + "env_vars": { + "BUZZ_AGENT_PROVIDER": "databricks", + "DATABRICKS_HOST": "https://dbc.example.com" + } + }]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ true, + ); + + let records = read_agents_json(dir.path()); + // Stale derived key must be removed. + assert!( + records[0]["env_vars"].get("BUZZ_AGENT_PROVIDER").is_none(), + "BUZZ_AGENT_PROVIDER must be stripped from env_vars" + ); + // Non-derived keys must be preserved. + assert_eq!( + records[0]["env_vars"]["DATABRICKS_HOST"], + "https://dbc.example.com" + ); +} + +#[test] +fn reconcile_databricks_v1_to_v2_strips_all_derived_keys_from_env_vars() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Brain", + "provider": "anthropic", + "model": "claude-opus-4-5", + "env_vars": { + "BUZZ_AGENT_PROVIDER": "anthropic", + "BUZZ_AGENT_MODEL": "claude-opus-4-5", + "GOOSE_PROVIDER": "anthropic", + "GOOSE_MODEL": "claude-opus-4-5", + "ANTHROPIC_API_KEY": "sk-test" + } + }]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ true, + ); + + let records = read_agents_json(dir.path()); + let env_vars = &records[0]["env_vars"]; + // All four derived keys must be stripped. + assert!(env_vars.get("BUZZ_AGENT_PROVIDER").is_none()); + assert!(env_vars.get("BUZZ_AGENT_MODEL").is_none()); + assert!(env_vars.get("GOOSE_PROVIDER").is_none()); + assert!(env_vars.get("GOOSE_MODEL").is_none()); + // Non-derived key must be preserved. + assert_eq!(env_vars["ANTHROPIC_API_KEY"], "sk-test"); +} + +#[test] +fn reconcile_databricks_v1_to_v2_handles_multiple_records_block_build() { + // On Block builds (rewrite_v1_provider=true): V1 provider is migrated and + // env_vars stripping applies to every record. + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([ + { + "name": "Agent A", + "provider": "databricks", + "env_vars": { "BUZZ_AGENT_PROVIDER": "databricks" } + }, + { + "name": "Agent B", + "provider": "anthropic", + "env_vars": { "BUZZ_AGENT_MODEL": "claude-3-5-sonnet" } + }, + { + "name": "Agent C", + "provider": "databricks_v2", + "env_vars": {} + } + ]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ true, + ); + + let records = read_agents_json(dir.path()); + // A: provider rewritten, stale env_var stripped. + assert_eq!(records[0]["provider"], "databricks_v2"); + assert!(records[0]["env_vars"].get("BUZZ_AGENT_PROVIDER").is_none()); + // B: provider untouched, stale BUZZ_AGENT_MODEL stripped. + assert_eq!(records[1]["provider"], "anthropic"); + assert!(records[1]["env_vars"].get("BUZZ_AGENT_MODEL").is_none()); + // C: V2 provider, no stale keys — unchanged. + assert_eq!(records[2]["provider"], "databricks_v2"); +} + +#[test] +fn reconcile_databricks_v1_to_v2_is_idempotent() { + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Brain", + "provider": "databricks", + "env_vars": { "BUZZ_AGENT_PROVIDER": "databricks" } + }]), + ); + let path = dir.path().join("agents/managed-agents.json"); + + reconcile_databricks_v1_to_v2_in_file(&path, /*rewrite_v1_provider=*/ true); + let after_first = std::fs::read_to_string(&path).unwrap(); + reconcile_databricks_v1_to_v2_in_file(&path, /*rewrite_v1_provider=*/ true); + let after_second = std::fs::read_to_string(&path).unwrap(); + + assert_eq!( + after_first, after_second, + "second pass must not modify the file" + ); +} + +#[test] +fn reconcile_databricks_v1_to_v2_preserves_non_databricks_providers() { + let dir = tempfile::tempdir().unwrap(); + let json = serde_json::json!([ + { "name": "A", "provider": "anthropic" }, + { "name": "B", "provider": "openai" }, + { "name": "C", "provider": "openai-compat" }, + ]); + write_agents_json(dir.path(), &json); + let path = dir.path().join("agents/managed-agents.json"); + let before = std::fs::read_to_string(&path).unwrap(); + + reconcile_databricks_v1_to_v2_in_file(&path, /*rewrite_v1_provider=*/ true); + + // No provider is modified, so the file content is identical. + assert_eq!(before, std::fs::read_to_string(&path).unwrap()); +} + +#[test] +fn reconcile_databricks_v1_to_v2_strips_derived_keys_from_keyless_persona_definition() { + // Folded persona definitions land in managed-agents.json without a + // "provider" key (they are keyless/definition records). Stale derived env + // keys in their env_vars must be stripped by the migration just like + // full agent records — persona env is merged after runtime metadata and + // can shadow structured fields at spawn time. + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + // No "provider" or "model" key — this is a folded persona definition. + "name": "Fizz", + "persona_id": "builtin:fizz", + "env_vars": { + "BUZZ_AGENT_PROVIDER": "databricks", + "BUZZ_AGENT_MODEL": "goose-claude-4-6-sonnet", + "DATABRICKS_HOST": "https://dbc.example.com" + } + }]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ true, + ); + + let records = read_agents_json(dir.path()); + let env_vars = &records[0]["env_vars"]; + // Derived keys stripped even though there is no top-level "provider" field. + assert!( + env_vars.get("BUZZ_AGENT_PROVIDER").is_none(), + "BUZZ_AGENT_PROVIDER must be stripped from keyless persona definition env_vars" + ); + assert!( + env_vars.get("BUZZ_AGENT_MODEL").is_none(), + "BUZZ_AGENT_MODEL must be stripped from keyless persona definition env_vars" + ); + // Non-derived key preserved. + assert_eq!(env_vars["DATABRICKS_HOST"], "https://dbc.example.com"); +} + +#[test] +fn reconcile_databricks_v1_to_v2_strips_derived_keys_case_insensitively() { + // The derived-key check is case-insensitive (matching is_derived_provider_model_key). + // A record with mixed-case variants must have those keys stripped. + let dir = tempfile::tempdir().unwrap(); + write_agents_json( + dir.path(), + &serde_json::json!([{ + "name": "Brain", + "provider": "databricks_v2", + "env_vars": { + "buzz_agent_provider": "databricks", + "Buzz_Agent_Model": "goose-claude-4-6-sonnet", + "DATABRICKS_HOST": "https://dbc.example.com" + } + }]), + ); + + reconcile_databricks_v1_to_v2_in_file( + &dir.path().join("agents/managed-agents.json"), + /*rewrite_v1_provider=*/ true, + ); + + let records = read_agents_json(dir.path()); + let env_vars = &records[0]["env_vars"]; + // Mixed-case derived keys must be stripped. + assert!(env_vars.get("buzz_agent_provider").is_none()); + assert!(env_vars.get("Buzz_Agent_Model").is_none()); + // Non-derived key preserved. + assert_eq!(env_vars["DATABRICKS_HOST"], "https://dbc.example.com"); +} diff --git a/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs b/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs index efb84daf59e..4a75f2eaa23 100644 --- a/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs +++ b/desktop/src/features/agents/ui/editAgentProviderDiscovery.test.mjs @@ -55,18 +55,36 @@ test("editAgent_providerFieldHidden_forBlankRuntime", () => { // ── Provider dropdown options for EditAgentProviderField ──────────────────── // -// The provider dropdown must always contain the well-known providers -// (databricks, databricks_v2, anthropic, openai, openai-compat) plus a -// default-provider fallback entry so users can clear a saved provider. - -test("editAgent_providerOptions_includesDatabricksProviders", () => { +// The provider dropdown contains the well-known providers +// (databricks_v2, anthropic, openai, openai-compat) plus a default-provider +// fallback entry so users can clear a saved provider. +// Note: bare "databricks" (V1 / Model Serving) is no longer offered as a +// fresh choice in the default picker — Block builds migrate it to V2 at boot +// and the picker would silently undo any intentional V1 selection. + +test("editAgent_providerOptions_includesDatabricksV2Provider", () => { const options = getPersonaProviderOptions("", "buzz-agent"); const ids = options.map((o) => o.id); - assert.ok(ids.includes("databricks"), "databricks must be a provider option"); assert.ok( ids.includes("databricks_v2"), "databricks_v2 must be a provider option", ); + assert.ok( + !ids.includes("databricks"), + "bare databricks (V1) must NOT be in the default provider list", + ); +}); + +test("editAgent_providerOptions_includesDatabricksV1AsCurrentIfSaved", () => { + // A record that already has provider="databricks" (OSS / pre-migration) + // must still show it in the dropdown as the current selection so it + // remains visible without offering it as a fresh default choice. + const options = getPersonaProviderOptions("databricks", "buzz-agent"); + const ids = options.map((o) => o.id); + assert.ok( + ids.includes("databricks"), + "databricks must appear as current provider when it is the saved value", + ); }); test("editAgent_providerOptions_includesDefaultEntry", () => { diff --git a/desktop/src/features/agents/ui/personaDialogPickers.tsx b/desktop/src/features/agents/ui/personaDialogPickers.tsx index c3b4cd07a88..1566be51ac3 100644 --- a/desktop/src/features/agents/ui/personaDialogPickers.tsx +++ b/desktop/src/features/agents/ui/personaDialogPickers.tsx @@ -16,7 +16,6 @@ export const NO_RUNTIME_DROPDOWN_VALUE = "__no_runtime__"; const KNOWN_LLM_PROVIDER_IDS = [ "anthropic", - "databricks", "databricks_v2", "openai", "openai-compat", @@ -50,8 +49,7 @@ const PERSONA_LLM_PROVIDER_OPTIONS: readonly PersonaModelOption[] = [ { id: "anthropic", label: "Anthropic" }, { id: "openai", label: "OpenAI" }, { id: "openai-compat", label: "OpenAI-compatible" }, - { id: "databricks", label: "Databricks" }, - { id: "databricks_v2", label: "Databricks v2" }, + { id: "databricks_v2", label: "Databricks v2 (AI Gateway)" }, ]; const PERSONA_MODEL_OPTIONS_BY_RUNTIME: Record< @@ -100,7 +98,8 @@ export function requiredCredentialEnvKeys( if (normalizedProvider === "openai") return ["OPENAI_COMPAT_API_KEY"]; if ( normalizedProvider === "databricks" || - normalizedProvider === "databricks_v2" + normalizedProvider === "databricks_v2" || + normalizedProvider === "databricks-v2" ) { // DATABRICKS_TOKEN is NOT required — OAuth PKCE is the normal path. return ["DATABRICKS_HOST"];