From 00c6a92c19add9d13e699875b047b2e398108d36 Mon Sep 17 00:00:00 2001 From: Brainy Bumble <0ed7657b57c0e8a9f5288390dd6c8d5d0a3a06abe9b01b9006814f52077d6cdf@buzz.block.builderlab.xyz> Date: Thu, 6 Aug 2026 00:23:00 -0500 Subject: [PATCH] feat(agents): make thinking effort configurable and portable in exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex thinking effort was unreachable from Buzz, and neither the agent nor the team JSON export carried effort or any environment configuration. Fix both, and fix the pre-existing bug that made the live-ACP effort tier dead code. Root cause of "there is no way to set Codex thinking effort": `reader.rs` matched session config options by `category == "effort"`, but no real adapter publishes that category. `codex-acp` 1.1.9 and `claude-agent-acp` 0.58.1 both emit `category == "thought_level"` (config ids `reasoning_effort` and `effort` respectively). The `AcpConfigOption` tier therefore never fired for any runtime and every agent fell through to `RespawnWithEnvVar` — or to `ReadOnly` for Codex, which had no thinking env var at all. Match `thought_level` and read the adapter-advertised `config_id` instead of hardcoding `"effort"`, the way `find_model_config_id` already does for the model field. Codex effort is settable via environment after all: the `codex-acp` adapter Buzz spawns ignores CLI args entirely and reads its configuration from the `CODEX_CONFIG` JSON env var. Probing the real adapter over ACP, `-c model_reasoning_effort=low` left `reasoning_effort` at `high`, while `CODEX_CONFIG={"model_reasoning_effort":"low"}` took effect. Declare that target in the runtime catalog as `thinking_config_json_env_var` + `thinking_config_json_key` so no per-runtime branch is needed in the UI or the snapshot code, and `build_codex_config_env` already deep-merges it into the generated network overlay so the value reaches the child process. Exports now carry a first-class scalar `thinking_effort` plus an allowlisted `portable_env`. `CODEX_CONFIG` itself is deliberately not allowlisted: it can carry `model_providers.*` auth headers, so only the extracted effort scalar is projected. Import re-applies the allowlist and the normal env validation, so a hand-edited snapshot cannot inject arbitrary environment, and the first-class effort field wins when it conflicts with `portable_env`. New fields are `#[serde(default, skip_serializing_if)]`, so v1 snapshots still decode and clean records emit no new keys. Several files were at the desktop file-size ratchet, so the new code is split into sibling modules rather than growing capped files: the runtime table, the catalog entry type (Rust and TS), the raw catalog bridge, and three test fixture/test files. Co-authored-by: Hardworking Honey Co-authored-by: Atish Patel Signed-off-by: Atish Patel --- crates/buzz-acp/src/acp.rs | 12 ++ .../src/commands/agent_config_tests.rs | 2 + .../src-tauri/src/commands/agent_discovery.rs | 34 +--- .../agent_discovery/custom_harness_entry.rs | 50 ++++++ .../src-tauri/src/commands/media_download.rs | 6 + .../src/commands/media_snapshot_png.rs | 2 + .../src/commands/personas/snapshot.rs | 2 + .../personas/snapshot/fidelity_tests.rs | 2 + .../src/commands/personas/snapshot/import.rs | 127 ++------------ .../personas/snapshot/import_avatar_tests.rs | 112 +++++++++++++ .../src/commands/personas/snapshot/tests.rs | 36 +++- .../src-tauri/src/commands/team_snapshot.rs | 28 +++- .../src/commands/team_snapshot/tests.rs | 64 +++++++ .../src/managed_agents/agent_snapshot.rs | 21 ++- .../managed_agents/agent_snapshot_envelope.rs | 2 + .../managed_agents/agent_snapshot_tests.rs | 36 ++++ .../managed_agents/config_bridge/reader.rs | 27 ++- .../config_bridge/reader_tests.rs | 45 +---- .../config_bridge/reader_thinking_tests.rs | 99 +++++++++++ .../src-tauri/src/managed_agents/discovery.rs | 155 ++--------------- .../discovery/known_runtimes.rs | 156 ++++++++++++++++++ .../src/managed_agents/discovery/presets.rs | 2 + .../discovery/runtime_metadata.rs | 5 + .../src-tauri/src/managed_agents/env_vars.rs | 100 +++++++++++ .../src/managed_agents/env_vars/tests.rs | 59 ++++++- .../src-tauri/src/managed_agents/readiness.rs | 93 +---------- .../managed_agents/readiness_test_runtimes.rs | 96 +++++++++++ desktop/src-tauri/src/managed_agents/types.rs | 49 +----- .../managed_agents/types/runtime_catalog.rs | 58 +++++++ desktop/src/features/agents/AGENTS.md | 14 +- .../agents/lib/agentConfigCore.test.mjs | 64 +++++++ .../features/agents/lib/agentConfigCore.ts | 124 +++++++++++++- .../agents/ui/EditAgentAdvancedFields.tsx | 71 +++++++- .../src/shared/api/runtimeCatalogBridge.ts | 79 +++++++++ desktop/src/shared/api/runtimeCatalogTypes.ts | 59 +++++++ desktop/src/shared/api/tauri.test.mjs | 26 +++ desktop/src/shared/api/tauri.ts | 73 +------- desktop/src/shared/api/types.ts | 52 +----- 38 files changed, 1445 insertions(+), 597 deletions(-) create mode 100644 desktop/src-tauri/src/commands/agent_discovery/custom_harness_entry.rs create mode 100644 desktop/src-tauri/src/commands/personas/snapshot/import_avatar_tests.rs create mode 100644 desktop/src-tauri/src/managed_agents/config_bridge/reader_thinking_tests.rs create mode 100644 desktop/src-tauri/src/managed_agents/discovery/known_runtimes.rs create mode 100644 desktop/src-tauri/src/managed_agents/readiness_test_runtimes.rs create mode 100644 desktop/src-tauri/src/managed_agents/types/runtime_catalog.rs create mode 100644 desktop/src/shared/api/runtimeCatalogBridge.ts create mode 100644 desktop/src/shared/api/runtimeCatalogTypes.ts diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 93109fa94d..c0e855c1f3 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -4501,6 +4501,18 @@ mod tests { assert_eq!(v["sandbox_workspace_write"]["network_access"], true); } + #[test] + fn build_codex_config_env_preserves_persona_reasoning_effort() { + let persona_cfg = r#"{"model_reasoning_effort":"high","other_setting":"keep"}"#; + let extra = env(&[("CODEX_CONFIG", persona_cfg), ("CODEX_CONFIG", GENERATED)]); + let merged = build_codex_config_env(&extra, None, true).unwrap().unwrap(); + let value: serde_json::Value = serde_json::from_str(&merged).unwrap(); + + assert_eq!(value["model_reasoning_effort"], "high"); + assert_eq!(value["other_setting"], "keep"); + assert_eq!(value["sandbox_workspace_write"]["network_access"], true); + } + #[test] fn build_codex_config_env_persona_keys_survive_merge() { // Persona has CODEX_CONFIG with unrelated keys; generated overlay must diff --git a/desktop/src-tauri/src/commands/agent_config_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index 5519153578..90c5692957 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -55,6 +55,8 @@ fn goose_runtime() -> &'static KnownAcpRuntime { config_file_format: Some("yaml"), supports_acp_native_config: true, thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + thinking_config_json_env_var: None, + thinking_config_json_key: None, max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), max_rounds_env_var: None, diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 9609db5f2d..1d793e3134 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -11,7 +11,9 @@ use crate::{ relay::query_relay, }; +mod custom_harness_entry; mod post_install_verification; +use custom_harness_entry::custom_harness_catalog_entry; fn active_installs() -> &'static std::sync::Mutex> { use std::collections::HashSet; @@ -96,9 +98,7 @@ pub async fn save_custom_harness( original_id: Option, app: tauri::AppHandle, ) -> Result { - use crate::managed_agents::{ - custom_harnesses, AcpAvailabilityStatus, AuthStatus, HarnessSource, - }; + use crate::managed_agents::{custom_harnesses, AcpAvailabilityStatus}; use tauri::Manager; // ── Phase 1: full validation before touching the filesystem ───────────── @@ -152,33 +152,13 @@ pub async fn save_custom_harness( let default_args = crate::managed_agents::normalize_agent_args(&definition.command, definition.args.clone()); - Ok(AcpRuntimeCatalogEntry { - id: definition.id, - label: definition.label, - avatar_url: String::new(), + Ok(custom_harness_catalog_entry( + definition, availability, - command: command_opt, + command_opt, binary_path, default_args, - mcp_command: None, - model_env_var: None, - provider_env_var: None, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - max_rounds_env_var: None, - install_hint: definition.install_hint, - install_instructions_url: definition.install_instructions_url, - can_auto_install: false, - requires_external_cli: false, - underlying_cli_path: None, - node_required: false, - auth_status: AuthStatus::NotApplicable, - login_hint: None, - source: HarnessSource::Custom, - definition_env: definition.env, - max_parallelism: crate::managed_agents::harness_max_parallelism(&definition.command), - }) + )) } /// Remove a user-defined harness definition from `/custom_harnesses/`. diff --git a/desktop/src-tauri/src/commands/agent_discovery/custom_harness_entry.rs b/desktop/src-tauri/src/commands/agent_discovery/custom_harness_entry.rs new file mode 100644 index 0000000000..5a949239fc --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_discovery/custom_harness_entry.rs @@ -0,0 +1,50 @@ +//! Catalog-entry projection for a user-defined harness, split out of +//! `agent_discovery.rs` to respect the repo's file-size ratchet. + +use crate::managed_agents::{ + custom_harnesses::HarnessDefinition, AcpAvailabilityStatus, AcpRuntimeCatalogEntry, AuthStatus, + HarnessSource, +}; + +/// Build the catalog row returned to the UI after a custom harness is saved. +/// +/// Custom harnesses declare no runtime capability metadata, so every env-var +/// target — including the structured thinking-config pair — is `None`; effort +/// for these entries can only come from the live ACP session. +pub(super) fn custom_harness_catalog_entry( + definition: HarnessDefinition, + availability: AcpAvailabilityStatus, + command_opt: Option, + binary_path: Option, + default_args: Vec, +) -> AcpRuntimeCatalogEntry { + AcpRuntimeCatalogEntry { + id: definition.id, + label: definition.label, + avatar_url: String::new(), + availability, + command: command_opt, + binary_path, + default_args, + mcp_command: None, + model_env_var: None, + provider_env_var: None, + thinking_env_var: None, + thinking_config_json_env_var: None, + thinking_config_json_key: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, + install_hint: definition.install_hint, + install_instructions_url: definition.install_instructions_url, + can_auto_install: false, + requires_external_cli: false, + underlying_cli_path: None, + node_required: false, + auth_status: AuthStatus::NotApplicable, + login_hint: None, + source: HarnessSource::Custom, + definition_env: definition.env, + max_parallelism: crate::managed_agents::harness_max_parallelism(&definition.command), + } +} diff --git a/desktop/src-tauri/src/commands/media_download.rs b/desktop/src-tauri/src/commands/media_download.rs index 7bc94da25d..733d467f60 100644 --- a/desktop/src-tauri/src/commands/media_download.rs +++ b/desktop/src-tauri/src/commands/media_download.rs @@ -618,6 +618,8 @@ mod tests { runtime: None, model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: None, respond_to: None, respond_to_allowlist: vec![], @@ -668,6 +670,8 @@ mod tests { runtime: None, model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: None, respond_to: None, respond_to_allowlist: vec![], @@ -714,6 +718,8 @@ mod tests { runtime: None, model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: None, respond_to: None, respond_to_allowlist: vec![], diff --git a/desktop/src-tauri/src/commands/media_snapshot_png.rs b/desktop/src-tauri/src/commands/media_snapshot_png.rs index bcaec6a592..0aa5600a2d 100644 --- a/desktop/src-tauri/src/commands/media_snapshot_png.rs +++ b/desktop/src-tauri/src/commands/media_snapshot_png.rs @@ -163,6 +163,8 @@ mod tests { runtime: Some("goose".to_string()), model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: Some(1), respond_to: None, respond_to_allowlist: vec![], diff --git a/desktop/src-tauri/src/commands/personas/snapshot.rs b/desktop/src-tauri/src/commands/personas/snapshot.rs index e7bd1597e6..e784b17fa2 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot.rs @@ -484,6 +484,8 @@ mod png_body_tests { runtime: None, model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: None, respond_to: None, respond_to_allowlist: vec![], diff --git a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs index b769d74d7b..54c89323ee 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs @@ -82,6 +82,8 @@ fn make_snapshot( runtime: None, model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: None, respond_to: None, respond_to_allowlist: vec![], diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index d7f0323304..0a5b74e7c8 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -18,8 +18,8 @@ use crate::{ decrypt_envelope, parse_chunk_payload, resolve_unlock_secret, ChunkPayload, LOCKED_CARD_REFUSAL, }, - load_managed_agents, load_personas, save_managed_agents, save_personas, AgentDefinition, - ManagedAgentRecord, RespondTo, + load_managed_agents, load_personas, portable_config_for_import, save_managed_agents, + save_personas, AgentDefinition, ManagedAgentRecord, RespondTo, }, relay::{effective_agent_relay_url, relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -487,6 +487,12 @@ pub async fn confirm_agent_snapshot_import( )?; let minted_parallelism = minted.parallelism; + let portable_env = portable_config_for_import( + snapshot.definition.runtime.as_deref(), + &snapshot.definition.portable_env, + snapshot.definition.thinking_effort.as_deref(), + )?; + // Profile metadata must contain a hosted URL. Inline avatar data can be far // larger than the relay's kind:0 content limit, so upload imported pixels // before minting or persisting the new agent. Failing here keeps import @@ -578,7 +584,7 @@ pub async fn confirm_agent_snapshot_import( source_team: None, source_team_persona_slug: None, catalog_source: None, - env_vars: std::collections::BTreeMap::new(), + env_vars: portable_env.clone(), respond_to: respond_to_wire.clone(), respond_to_allowlist: minted.respond_to_allowlist.clone(), parallelism: minted_parallelism, @@ -620,7 +626,7 @@ pub async fn confirm_agent_snapshot_import( model: snapshot.definition.model.clone(), provider: snapshot.definition.provider.clone(), persona_source_version: None, - env_vars: std::collections::BTreeMap::new(), + env_vars: portable_env.clone(), start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, @@ -885,113 +891,8 @@ mod egress_guard_tests { } } +// Avatar-materialization tests live in a sibling file so this module stays under +// the desktop file-size ratchet. #[cfg(test)] -mod import_avatar_tests { - use super::materialize_import_avatar; - use std::cell::Cell; - - #[tokio::test] - async fn inline_avatar_is_uploaded_and_replaced_with_hosted_url() { - let uploaded = Cell::new(false); - let result = materialize_import_avatar( - Some("data:image/png;base64,iVBORw0KGgo="), - Some("https://sender.invalid/avatar.png"), - |bytes| { - uploaded.set(true); - async move { - assert_eq!(bytes, b"\x89PNG\r\n\x1a\n"); - Ok("https://relay.example/media/avatar.png".to_string()) - } - }, - ) - .await - .unwrap(); - - assert!(uploaded.get()); - assert_eq!( - result.as_deref(), - Some("https://relay.example/media/avatar.png") - ); - } - - #[tokio::test] - async fn hosted_avatar_skips_upload() { - let result = - materialize_import_avatar(None, Some("https://sender.example/avatar.png"), |_| async { - panic!("hosted avatars must not be uploaded") - }) - .await - .unwrap(); - - assert_eq!(result.as_deref(), Some("https://sender.example/avatar.png")); - } - - #[tokio::test] - async fn relay_sized_inline_avatar_becomes_bounded_signed_profile() { - use base64::{engine::general_purpose::STANDARD, Engine}; - use image::ImageEncoder; - use nostr::JsonUtil; - - let mut pixels = vec![0_u8; 512 * 512 * 4]; - let mut seed = 0x1234_5678_u32; - for byte in &mut pixels { - seed ^= seed << 13; - seed ^= seed >> 17; - seed ^= seed << 5; - *byte = seed as u8; - } - let mut source = Vec::new(); - image::codecs::png::PngEncoder::new(&mut source) - .write_image(&pixels, 512, 512, image::ExtendedColorType::Rgba8) - .unwrap(); - assert!(source.len() > 256 * 1024); - let data_url = format!("data:image/png;base64,{}", STANDARD.encode(&source)); - assert!(data_url.len() > 256 * 1024); - - let avatar = materialize_import_avatar(Some(&data_url), None, |bytes| async move { - let mime = crate::commands::media::detect_and_validate_mime(&bytes)?; - assert_eq!(mime, "image/png"); - let sanitized = crate::commands::media::sanitize_image_for_upload(bytes, &mime)?; - image::load_from_memory(&sanitized).map_err(|error| error.to_string())?; - Ok("https://relay.example/media/avatar.png".to_string()) - }) - .await - .unwrap() - .unwrap(); - - let event = - crate::events::build_profile(Some("Imported agent"), None, Some(&avatar), None, None) - .unwrap() - .sign_with_keys(&nostr::Keys::generate()) - .unwrap(); - assert!(event.content.len() < 64 * 1024); - assert!(!event.content.contains("data:image/")); - assert!(event - .content - .contains("https://relay.example/media/avatar.png")); - assert!(event.as_json().len() < 256 * 1024); - } - - #[tokio::test] - async fn upload_failure_aborts_avatar_materialization() { - let result = materialize_import_avatar( - Some("data:image/png;base64,iVBORw0KGgo="), - None, - |_| async { Err("relay upload failed".to_string()) }, - ) - .await; - - assert_eq!(result.unwrap_err(), "relay upload failed"); - } - - #[tokio::test] - async fn malformed_inline_avatar_fails_before_upload() { - let result = - materialize_import_avatar(Some("data:image/png;base64,not-base64!"), None, |_| async { - panic!("malformed avatars must not be uploaded") - }) - .await; - - assert_eq!(result.unwrap_err(), "Snapshot avatar data is malformed."); - } -} +#[path = "import_avatar_tests.rs"] +mod import_avatar_tests; diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import_avatar_tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/import_avatar_tests.rs new file mode 100644 index 0000000000..d4bfcff8f8 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/snapshot/import_avatar_tests.rs @@ -0,0 +1,112 @@ +//! Avatar-materialization tests for `personas/snapshot/import.rs`, split into a +//! sibling file to respect the repo's file-size ratchet. +//! +//! Declared as `mod import_avatar_tests` from `import.rs`. + +use super::materialize_import_avatar; +use std::cell::Cell; + +#[tokio::test] +async fn inline_avatar_is_uploaded_and_replaced_with_hosted_url() { + let uploaded = Cell::new(false); + let result = materialize_import_avatar( + Some("data:image/png;base64,iVBORw0KGgo="), + Some("https://sender.invalid/avatar.png"), + |bytes| { + uploaded.set(true); + async move { + assert_eq!(bytes, b"\x89PNG\r\n\x1a\n"); + Ok("https://relay.example/media/avatar.png".to_string()) + } + }, + ) + .await + .unwrap(); + + assert!(uploaded.get()); + assert_eq!( + result.as_deref(), + Some("https://relay.example/media/avatar.png") + ); +} + +#[tokio::test] +async fn hosted_avatar_skips_upload() { + let result = + materialize_import_avatar(None, Some("https://sender.example/avatar.png"), |_| async { + panic!("hosted avatars must not be uploaded") + }) + .await + .unwrap(); + + assert_eq!(result.as_deref(), Some("https://sender.example/avatar.png")); +} + +#[tokio::test] +async fn relay_sized_inline_avatar_becomes_bounded_signed_profile() { + use base64::{engine::general_purpose::STANDARD, Engine}; + use image::ImageEncoder; + use nostr::JsonUtil; + + let mut pixels = vec![0_u8; 512 * 512 * 4]; + let mut seed = 0x1234_5678_u32; + for byte in &mut pixels { + seed ^= seed << 13; + seed ^= seed >> 17; + seed ^= seed << 5; + *byte = seed as u8; + } + let mut source = Vec::new(); + image::codecs::png::PngEncoder::new(&mut source) + .write_image(&pixels, 512, 512, image::ExtendedColorType::Rgba8) + .unwrap(); + assert!(source.len() > 256 * 1024); + let data_url = format!("data:image/png;base64,{}", STANDARD.encode(&source)); + assert!(data_url.len() > 256 * 1024); + + let avatar = materialize_import_avatar(Some(&data_url), None, |bytes| async move { + let mime = crate::commands::media::detect_and_validate_mime(&bytes)?; + assert_eq!(mime, "image/png"); + let sanitized = crate::commands::media::sanitize_image_for_upload(bytes, &mime)?; + image::load_from_memory(&sanitized).map_err(|error| error.to_string())?; + Ok("https://relay.example/media/avatar.png".to_string()) + }) + .await + .unwrap() + .unwrap(); + + let event = + crate::events::build_profile(Some("Imported agent"), None, Some(&avatar), None, None) + .unwrap() + .sign_with_keys(&nostr::Keys::generate()) + .unwrap(); + assert!(event.content.len() < 64 * 1024); + assert!(!event.content.contains("data:image/")); + assert!(event + .content + .contains("https://relay.example/media/avatar.png")); + assert!(event.as_json().len() < 256 * 1024); +} + +#[tokio::test] +async fn upload_failure_aborts_avatar_materialization() { + let result = materialize_import_avatar( + Some("data:image/png;base64,iVBORw0KGgo="), + None, + |_| async { Err("relay upload failed".to_string()) }, + ) + .await; + + assert_eq!(result.unwrap_err(), "relay upload failed"); +} + +#[tokio::test] +async fn malformed_inline_avatar_fails_before_upload() { + let result = + materialize_import_avatar(Some("data:image/png;base64,not-base64!"), None, |_| async { + panic!("malformed avatars must not be uploaded") + }) + .await; + + assert_eq!(result.unwrap_err(), "Snapshot avatar data is malformed."); +} diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index c453b09a9d..3082ba6399 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -9,7 +9,7 @@ use crate::managed_agents::{ AgentSnapshot, AgentSnapshotDefinition, AgentSnapshotMemory, AgentSnapshotMemoryEntry, AgentSnapshotProfile, FORMAT_DISCRIMINATOR, FORMAT_VERSION, }, - BackendKind, ManagedAgentRecord, RespondTo, + portable_config_for_import, BackendKind, ManagedAgentRecord, RespondTo, }; use std::collections::BTreeMap; @@ -102,6 +102,8 @@ fn make_snapshot( runtime: None, model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: None, respond_to: None, respond_to_allowlist: vec![], @@ -571,6 +573,38 @@ fn import_preview_includes_exported_definition_metadata() { assert_eq!(preview.runtime.as_deref(), Some("goose")); } +// ── Import: portable runtime configuration ────────────────────────────── + +#[test] +fn agent_snapshot_import_configuration_restores_codex_effort_and_safe_env_only() { + let snapshot = AgentSnapshotDefinition { + runtime: Some("codex".to_string()), + thinking_effort: Some("high".to_string()), + portable_env: BTreeMap::from([ + ("CLAUDE_CODE_EFFORT_LEVEL".to_string(), "medium".to_string()), + ("OPENAI_API_KEY".to_string(), "secret".to_string()), + ]), + ..make_snapshot(MemoryLevel::None, vec![]).definition + }; + + let env_vars = portable_config_for_import( + snapshot.runtime.as_deref(), + &snapshot.portable_env, + snapshot.thinking_effort.as_deref(), + ) + .unwrap(); + + assert_eq!( + env_vars.get("CODEX_CONFIG").map(String::as_str), + Some(r#"{"model_reasoning_effort":"high"}"#) + ); + assert_eq!( + env_vars.get("CLAUDE_CODE_EFFORT_LEVEL").map(String::as_str), + Some("medium") + ); + assert!(!env_vars.contains_key("OPENAI_API_KEY")); +} + // ── Import: resolve_snapshot_import_behavior — the production selection path // // All tests below call `resolve_snapshot_import_behavior` directly. This is diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 97cd11933d..9a38c56a2c 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -17,8 +17,9 @@ use crate::{ }, managed_agents::{ agent_snapshot::{build_snapshot, AgentSnapshot, AgentSnapshotMemoryEntry, MemoryLevel}, - load_managed_agents, load_personas, load_teams, load_teams_readonly, save_managed_agents, - save_personas, save_teams, AgentDefinition, ManagedAgentRecord, TeamRecord, + load_managed_agents, load_personas, load_teams, load_teams_readonly, + portable_config_for_import, save_managed_agents, save_personas, save_teams, + AgentDefinition, ManagedAgentRecord, TeamRecord, }, relay::{effective_agent_relay_url, relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -118,6 +119,12 @@ fn definition_from_snapshot( let respond_to = (behavior.respond_to != crate::managed_agents::RespondTo::default()) .then(|| behavior.respond_to.as_str().to_string()); + let env_vars = portable_config_for_import( + member.definition.runtime.as_deref(), + &member.definition.portable_env, + member.definition.thinking_effort.as_deref(), + )?; + Ok(AgentDefinition { id: Uuid::new_v4().to_string(), display_name: member.profile.display_name.trim().to_string(), @@ -133,7 +140,7 @@ fn definition_from_snapshot( source_team: None, source_team_persona_slug: None, catalog_source: None, - env_vars: Default::default(), + env_vars, respond_to, respond_to_allowlist: behavior.respond_to_allowlist, parallelism: behavior.parallelism, @@ -299,8 +306,14 @@ fn build_team_export_snapshot( _ => (MemoryLevel::None, Vec::new()), }; + let snapshot_record = instance.cloned().unwrap_or_else(|| { + let mut record = persona.clone().into_agent_record(); + record.env_vars = persona.env_vars.clone(); + record + }); + Ok(build_snapshot( - &persona.clone().into_agent_record(), + &snapshot_record, effective_level, entries, None, @@ -520,6 +533,11 @@ pub async fn confirm_team_snapshot_import( let mut minted: Vec = Vec::with_capacity(snapshot.members.len()); for (member, definition) in snapshot.members.iter().zip(definitions) { + let portable_env = portable_config_for_import( + member.definition.runtime.as_deref(), + &member.definition.portable_env, + member.definition.thinking_effort.as_deref(), + )?; let display_name = definition.display_name.clone(); let effective_avatar_url = effective_avatar(member); let respond_to_wire = definition.respond_to.clone(); @@ -573,7 +591,7 @@ pub async fn confirm_team_snapshot_import( model: member.definition.model.clone(), provider: member.definition.provider.clone(), persona_source_version: None, - env_vars: std::collections::BTreeMap::new(), + env_vars: portable_env, start_on_app_launch: false, auto_restart_on_config_change: true, runtime_pid: None, diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index c9a6d8812a..1573a57828 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -18,6 +18,8 @@ fn member(name: &str) -> AgentSnapshot { runtime: Some("goose".to_string()), model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: Some(2), respond_to: Some("allowlist".to_string()), respond_to_allowlist: vec!["ab".repeat(32)], @@ -272,6 +274,44 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { assert_eq!(snap_none.members[0].memory.level, MemoryLevel::None); assert!(snap_none.members[0].memory.entries.is_empty()); + // Per-instance configuration wins over the definition-derived fallback. + let mut configured_instance = instance.clone(); + configured_instance.runtime = Some("codex".to_string()); + configured_instance.env_vars.insert( + "CODEX_CONFIG".to_string(), + r#"{"model_reasoning_effort":"high"}"#.to_string(), + ); + configured_instance + .env_vars + .insert("CLAUDE_CODE_EFFORT_LEVEL".to_string(), "medium".to_string()); + configured_instance + .env_vars + .insert("OPENAI_API_KEY".to_string(), "must-not-export".to_string()); + let configured = build_team_export_snapshot( + &team, + &definitions, + std::slice::from_ref(&configured_instance), + MemoryLevel::None, + &memory_map, + ) + .unwrap(); + assert_eq!( + configured.members[0].definition.thinking_effort.as_deref(), + Some("high") + ); + assert_eq!( + configured.members[0] + .definition + .portable_env + .get("CLAUDE_CODE_EFFORT_LEVEL") + .map(String::as_str), + Some("medium") + ); + assert!(!configured.members[0] + .definition + .portable_env + .contains_key("OPENAI_API_KEY")); + // No instance → entries stripped even with Everything level. let snap_no_instance = build_team_export_snapshot( &team, @@ -329,6 +369,30 @@ fn team_import_definitions_are_built_for_all_members() { assert_eq!(definitions[0].system_prompt, "Alice prompt"); } +#[test] +fn team_import_definitions_restore_safe_portable_env_and_drop_secrets() { + let mut configured_member = member("Codex"); + configured_member.definition.runtime = Some("codex".to_string()); + configured_member.definition.thinking_effort = Some("high".to_string()); + configured_member.definition.portable_env = std::collections::BTreeMap::from([ + ("CLAUDE_CODE_EFFORT_LEVEL".to_string(), "medium".to_string()), + ("OPENAI_API_KEY".to_string(), "secret".to_string()), + ]); + + let definitions = + build_import_definitions(&snapshot(vec![configured_member]), false, "now").unwrap(); + + assert_eq!(definitions.len(), 1); + assert_eq!( + definitions[0] + .env_vars + .get("CLAUDE_CODE_EFFORT_LEVEL") + .map(String::as_str), + Some("medium") + ); + assert!(!definitions[0].env_vars.contains_key("OPENAI_API_KEY")); +} + #[test] fn team_import_keeps_or_clears_every_member_allowlist_with_one_toggle() { let source = snapshot(vec![member("Alice"), member("Bob")]); diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs index 7c08e7095f..14fdd21d85 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs @@ -21,7 +21,7 @@ //! The following fields are NEVER serialized: //! - `private_key_nsec` / any private key material //! - `auth_tag` (NIP-OA) -//! - `env_vars` (API keys / credentials) +//! - raw `env_vars` (API keys / credentials); only an explicit safe allowlist is projected //! - `relay_url` (machine-local endpoint) //! - `acp_command` / `agent_command` / `agent_command_override` / `agent_args` //! (machine-local harness paths) @@ -43,9 +43,12 @@ use base64::{engine::general_purpose::STANDARD, Engine as _}; use png::{BitDepth, ColorType, Decoder, Encoder}; use serde::{Deserialize, Serialize}; -use std::io::Cursor; +use std::{collections::BTreeMap, io::Cursor}; -use crate::managed_agents::types::ManagedAgentRecord; +use crate::managed_agents::{ + env_vars::{portable_env_for_export, thinking_effort_from_record}, + types::ManagedAgentRecord, +}; // ── Constants ──────────────────────────────────────────────────────────────── @@ -103,6 +106,16 @@ pub struct AgentSnapshotDefinition { pub model: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub provider: Option, + /// Harness-neutral thinking/effort value. `portable_env` retains the + /// runtime-specific setting where an environment variable is supported; + /// this field also carries Codex's structured configuration. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub thinking_effort: Option, + /// Non-secret environment-backed configuration. This is a projection of + /// `env_vars`, never the raw map: only keys approved by + /// `env_vars::is_safe_to_reveal` are serialized. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub portable_env: BTreeMap, #[serde(default, skip_serializing_if = "Option::is_none")] pub parallelism: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -204,6 +217,8 @@ pub fn build_snapshot( runtime: record.runtime.clone(), model: record.model.clone(), provider: record.provider.clone(), + thinking_effort: thinking_effort_from_record(record), + portable_env: portable_env_for_export(&record.env_vars), parallelism: record.definition_parallelism.or(Some(record.parallelism)), respond_to: record.definition_respond_to.clone(), respond_to_allowlist: record.definition_respond_to_allowlist.clone(), diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs index 8508c27073..4d249b10ef 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs @@ -337,6 +337,8 @@ mod tests { runtime: None, model: None, provider: None, + thinking_effort: None, + portable_env: Default::default(), parallelism: Some(1), respond_to: None, respond_to_allowlist: Vec::new(), diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs index b4492418e5..4354a0f2db 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs @@ -533,6 +533,8 @@ fn definition_fields_present_in_snapshot() { assert_eq!(snapshot.definition.runtime.as_deref(), Some("goose")); assert_eq!(snapshot.definition.model.as_deref(), Some("claude-opus-4")); assert_eq!(snapshot.definition.provider.as_deref(), Some("anthropic")); + assert!(snapshot.definition.thinking_effort.is_none()); + assert!(snapshot.definition.portable_env.is_empty()); assert_eq!(snapshot.definition.name_pool, vec!["Alice", "Bob"]); // definition_respond_to maps to respond_to in the snapshot definition assert_eq!(snapshot.definition.respond_to.as_deref(), Some("allowlist")); @@ -540,6 +542,40 @@ fn definition_fields_present_in_snapshot() { assert!(!snapshot.definition.respond_to_allowlist.is_empty()); } +#[test] +fn snapshot_exports_only_safe_env_and_codex_effort() { + let mut record = minimal_record(); + record.runtime = Some("codex".to_string()); + record.env_vars.insert( + "CODEX_CONFIG".to_string(), + r#"{"model_reasoning_effort":"high","other_setting":"preserved"}"#.to_string(), + ); + record + .env_vars + .insert("CLAUDE_CODE_EFFORT_LEVEL".to_string(), "medium".to_string()); + record + .env_vars + .insert("OPENAI_API_KEY".to_string(), "must-not-export".to_string()); + + let snapshot = build_snapshot(&record, MemoryLevel::None, vec![], None); + assert_eq!(snapshot.definition.thinking_effort.as_deref(), Some("high")); + assert_eq!( + snapshot + .definition + .portable_env + .get("CLAUDE_CODE_EFFORT_LEVEL") + .map(String::as_str), + Some("medium") + ); + assert!(!snapshot + .definition + .portable_env + .contains_key("OPENAI_API_KEY")); + let json = snapshot_json_string(&record); + assert!(!json.contains("must-not-export")); + assert!(!json.contains("unrelated=value")); +} + #[test] fn profile_fields_present_in_snapshot() { let record = minimal_record(); diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index c51f325cf3..289b80d317 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -49,7 +49,7 @@ pub(crate) fn read_config_surface( .or_else(|| find_config_option_value(c, "model")) }); let acp_mode = session_cache.and_then(|c| find_config_option_value(c, "mode")); - let acp_effort = session_cache.and_then(|c| find_config_option_value(c, "effort")); + let acp_effort = session_cache.and_then(|c| find_config_option_value(c, "thought_level")); let model_overridden = session_cache.is_some_and(|c| c.model_overridden); @@ -514,10 +514,15 @@ fn build_thinking_field( ]; let (value, origin, overridden_value, overridden_origin) = resolve_with_override(tiers_list)?; - let write_via = if !is_pre_spawn && has_config_option(session_cache, "effort") { - ConfigWriteMechanism::AcpSetConfigOption { - config_id: "effort".to_string(), - } + let write_via = if !is_pre_spawn { + find_thinking_config_id(session_cache) + .map(|config_id| ConfigWriteMechanism::AcpSetConfigOption { config_id }) + .unwrap_or_else(|| match thinking_env_var { + Some(env_key) => ConfigWriteMechanism::RespawnWithEnvVar { + env_key: env_key.to_string(), + }, + None => ConfigWriteMechanism::ReadOnly, + }) } else if let Some(env_key) = thinking_env_var { ConfigWriteMechanism::RespawnWithEnvVar { env_key: env_key.to_string(), @@ -694,6 +699,18 @@ fn find_model_config_id(cache: Option<&SessionConfigCache>) -> Option { }) } +/// ACP adapters expose effort as the `thought_level` category, but their +/// protocol config IDs differ (`reasoning_effort` for Codex, `effort` for +/// Claude). Return the advertised config ID instead of inventing one. +fn find_thinking_config_id(cache: Option<&SessionConfigCache>) -> Option { + cache.and_then(|c| { + c.config_options + .iter() + .find(|o| o.category.as_deref() == Some("thought_level")) + .map(|o| o.config_id.clone()) + }) +} + fn resolve_tilde(path: &str) -> String { if let Some(rest) = path.strip_prefix("~/") { if let Some(home) = dirs::home_dir() { diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 62caffeb2e..7cfd550c6b 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -54,6 +54,8 @@ fn test_runtime() -> &'static KnownAcpRuntime { config_file_format: Some("yaml"), supports_acp_native_config: true, thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + thinking_config_json_env_var: None, + thinking_config_json_key: None, max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), max_rounds_env_var: None, @@ -643,6 +645,8 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { config_file_format: None, supports_acp_native_config: false, thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), + thinking_config_json_env_var: None, + thinking_config_json_key: None, max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), @@ -894,43 +898,6 @@ fn no_effort_anywhere_yields_no_thinking_effort_field() { ); } -/// AC-5 (conflicting-ACP): inherited effort set (global=high) + live ACP effort=low -/// → ACP wins as primary (AcpConfigOption), global is the overridden secondary. -#[test] -fn acp_effort_wins_over_inherited_global_effort_as_secondary() { - let record = test_record(); - let runtime = buzz_agent_rt(); - let cache = SessionConfigCache { - config_options: vec![AcpConfigOptionEntry { - config_id: "effort".to_string(), - category: Some("effort".to_string()), - display_name: Some("Effort".to_string()), - current_value: Some("low".to_string()), - options: vec![], - }], - available_modes: vec![], - available_models: vec![], - current_model: None, - model_overridden: false, - goose_native_config: None, - captured_at: "".to_string(), - }; - let tiers = global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "high"); - - let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers); - - let effort = surface - .normalized - .thinking_effort - .expect("effort must surface from ACP tier"); - // Live ACP value wins. - assert_eq!(effort.value.as_deref(), Some("low")); - assert_eq!(effort.origin, ConfigOrigin::AcpConfigOption); - // Global is surfaced as the overridden baseline. - assert_eq!(effort.overridden_value.as_deref(), Some("high")); - assert_eq!(effort.overridden_origin, Some(ConfigOrigin::GlobalDefault)); -} - // ── Numerics inheritance tests ──────────────────────────────────────────────── // // max_output_tokens and context_limit gain persona/global tiers. @@ -948,6 +915,8 @@ fn numeric_max_tokens_inherits_from_global_env() { assert_eq!(field.origin, ConfigOrigin::GlobalDefault); } -// ── Extended tests (split file to respect line-count ratchet) ──────────────── +// ── Extended tests (split files to respect line-count ratchet) ─────────────── #[path = "reader_tests_ext.rs"] mod ext; +#[path = "reader_thinking_tests.rs"] +mod thinking; diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_thinking_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_thinking_tests.rs new file mode 100644 index 0000000000..cb6e4becb0 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_thinking_tests.rs @@ -0,0 +1,99 @@ +//! Thinking-effort tests for `config_bridge/reader.rs` — the live-ACP +//! `thought_level` tier and its env-var fallbacks. Split out of +//! `reader_tests.rs` to respect the repo's file-size ratchet. +//! +//! Included as `mod thinking` inside `reader_tests.rs`, so `use super::*` +//! gives access to all helpers and types from that module. + +use super::*; + +/// AC-5 (conflicting-ACP): inherited effort set (global=high) + live ACP effort=low +/// → ACP wins as primary (AcpConfigOption), global is the overridden secondary. +#[test] +fn acp_effort_wins_over_inherited_global_effort_as_secondary() { + let record = test_record(); + let runtime = buzz_agent_rt(); + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Effort".to_string()), + current_value: Some("low".to_string()), + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + let tiers = global_env_tiers("BUZZ_AGENT_THINKING_EFFORT", "high"); + + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &tiers); + + let effort = surface + .normalized + .thinking_effort + .expect("effort must surface from ACP tier"); + // Live ACP value wins. + assert_eq!(effort.value.as_deref(), Some("low")); + assert_eq!(effort.origin, ConfigOrigin::AcpConfigOption); + // Global is surfaced as the overridden baseline. + assert_eq!(effort.overridden_value.as_deref(), Some("high")); + assert_eq!(effort.overridden_origin, Some(ConfigOrigin::GlobalDefault)); + assert!(matches!( + effort.write_via, + ConfigWriteMechanism::AcpSetConfigOption { ref config_id } if config_id == "effort" + )); +} + +#[test] +fn codex_thought_level_uses_advertised_reasoning_effort_config_id() { + let record = test_record(); + let runtime = crate::managed_agents::known_acp_runtime_exact("codex").unwrap(); + let cache = SessionConfigCache { + config_options: vec![AcpConfigOptionEntry { + config_id: "reasoning_effort".to_string(), + category: Some("thought_level".to_string()), + display_name: Some("Reasoning effort".to_string()), + current_value: Some("high".to_string()), + options: vec![], + }], + available_modes: vec![], + available_models: vec![], + current_model: None, + model_overridden: false, + goose_native_config: None, + captured_at: "".to_string(), + }; + + let surface = read_config_surface(&record, Some(runtime), Some(&cache), &no_tiers()); + let effort = surface.normalized.thinking_effort.unwrap(); + assert_eq!(effort.value.as_deref(), Some("high")); + assert_eq!(effort.origin, ConfigOrigin::AcpConfigOption); + assert!(matches!( + effort.write_via, + ConfigWriteMechanism::AcpSetConfigOption { ref config_id } + if config_id == "reasoning_effort" + )); +} + +#[test] +fn pre_spawn_effort_uses_direct_env_fallback() { + let mut record = test_record(); + record + .env_vars + .insert("GOOSE_THINKING_EFFORT".to_string(), "medium".to_string()); + + let effort = read_config_surface(&record, Some(test_runtime()), None, &no_tiers()) + .normalized + .thinking_effort + .unwrap(); + assert_eq!(effort.value.as_deref(), Some("medium")); + assert!(matches!( + effort.write_via, + ConfigWriteMechanism::RespawnWithEnvVar { ref env_key } + if env_key == "GOOSE_THINKING_EFFORT" + )); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index bc0e3a6cda..d331ff423c 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -13,6 +13,14 @@ mod presets; mod runtime_metadata; #[macro_use] mod windows_install; +mod known_runtimes; +use known_runtimes::KNOWN_ACP_RUNTIMES; +// Avatar URLs live with the runtime table; the discovery tests assert against +// them, so re-export for that consumer only (no non-test reader here). +#[cfg(test)] +pub(crate) use known_runtimes::{ + BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, +}; pub(crate) use presets::{ canonical_harness_command, command_for_runtime_id, preset_harness_definitions, preset_harness_ids, @@ -20,11 +28,6 @@ pub(crate) use presets::{ use presets::{preset_catalog_entry, PRESET_HARNESSES}; pub(crate) use runtime_metadata::KnownAcpRuntime; -const GOOSE_AVATAR_URL: &str = "https://goose-docs.ai/img/logo_dark.png"; -const CLAUDE_CODE_AVATAR_URL: &str = "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default"; -const CODEX_AVATAR_URL: &str = "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default"; -const BUZZ_AGENT_AVATAR_URL: &str = - "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png"; fn common_binary_paths() -> &'static [PathBuf] { static PATHS: OnceLock> = OnceLock::new(); PATHS.get_or_init(|| { @@ -75,144 +78,6 @@ fn common_binary_paths() -> &'static [PathBuf] { }) } -const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ - KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: GOOSE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("goose"), - cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], - // Goose's stable release currently publishes only the Unix installer; - // its official Windows instructions intentionally point at this main-branch script. - cli_install_commands_windows: &[windows_install_command!("goose", "https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1", "$env:CONFIGURE='false'; ")], - adapter_install_commands: &[], - cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", - adapter_install_instructions_url: "", - cli_install_hint: "Buzz talks to Goose through the Goose CLI.", - adapter_install_hint: "", - skill_dir: Some(".goose/skills"), - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[("GOOSE_MODE", "auto")], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - max_rounds_env_var: None, - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, - KnownAcpRuntime { - id: "claude", - label: "Claude Code", - commands: &["claude-agent-acp", "claude-code-acp"], - aliases: &["claude-code", "claudecode"], - avatar_url: CLAUDE_CODE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("claude"), - cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], - cli_install_commands_windows: &[windows_install_command!("claude", "https://claude.ai/install.ps1")], - adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], - cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", - cli_install_hint: "Buzz talks to Claude Code through the Claude Code CLI.", - adapter_install_hint: "Buzz talks to the Claude Code CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/claude-agent-acp.", - skill_dir: Some(".claude/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: true, - default_env: &[], - config_file_path: Some("~/.claude/settings.json"), - config_file_format: Some("json"), - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run the Claude CLI to complete authentication."), - auth_probe_args: Some(&["claude", "auth", "status"]), - }, - KnownAcpRuntime { - id: "codex", - label: "Codex", - commands: &["codex-acp"], - aliases: &[], - avatar_url: CODEX_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: false, - underlying_cli: Some("codex"), - cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], - cli_install_commands_windows: &[windows_install_command!("codex", "https://chatgpt.com/codex/install.ps1")], - adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], - cli_install_instructions_url: "https://developers.openai.com/codex/cli/", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", - cli_install_hint: "Buzz talks to Codex through the Codex CLI.", - adapter_install_hint: "Buzz talks to the Codex CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/codex-acp.", - skill_dir: Some(".codex/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.codex/config.toml"), - config_file_format: Some("toml"), - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run `codex login` to authenticate."), - // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. - auth_probe_args: Some(&["codex", "login", "status"]), - }, - KnownAcpRuntime { - id: "buzz-agent", - label: "Buzz Agent", - commands: &["buzz-agent"], - aliases: &[], - avatar_url: BUZZ_AGENT_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: true, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "https://github.com/block/buzz", - adapter_install_instructions_url: "https://github.com/block/buzz", - cli_install_hint: "Ships with the Buzz desktop app.", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: true, - model_env_var: Some("BUZZ_AGENT_MODEL"), - provider_env_var: Some("BUZZ_AGENT_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: None, - config_file_format: None, - supports_acp_native_config: false, - thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), - max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), - context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), - max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, -]; - /// Skill discovery directories declared by known runtimes. pub(crate) fn known_skill_dirs() -> impl Iterator { KNOWN_ACP_RUNTIMES.iter().filter_map(|p| p.skill_dir) @@ -1390,6 +1255,8 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr model_env_var: runtime.model_env_var.map(str::to_string), provider_env_var: runtime.provider_env_var.map(str::to_string), thinking_env_var: runtime.thinking_env_var.map(str::to_string), + thinking_config_json_env_var: runtime.thinking_config_json_env_var.map(str::to_string), + thinking_config_json_key: runtime.thinking_config_json_key.map(str::to_string), max_tokens_env_var: runtime.max_tokens_env_var.map(str::to_string), context_limit_env_var: runtime.context_limit_env_var.map(str::to_string), max_rounds_env_var: runtime.max_rounds_env_var.map(str::to_string), @@ -1551,6 +1418,8 @@ pub fn discover_acp_runtimes_from( model_env_var: None, provider_env_var: None, thinking_env_var: None, + thinking_config_json_env_var: None, + thinking_config_json_key: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/known_runtimes.rs b/desktop/src-tauri/src/managed_agents/discovery/known_runtimes.rs new file mode 100644 index 0000000000..0222a10fb7 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/known_runtimes.rs @@ -0,0 +1,156 @@ +//! The compiled-in ACP runtime table and its avatar constants, split out of +//! `discovery.rs` to respect the repo's file-size ratchet. + +use super::runtime_metadata::KnownAcpRuntime; + +pub(crate) const GOOSE_AVATAR_URL: &str = "https://goose-docs.ai/img/logo_dark.png"; +pub(crate) const CLAUDE_CODE_AVATAR_URL: &str = "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default"; +pub(crate) const CODEX_AVATAR_URL: &str = "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default"; +pub(crate) const BUZZ_AGENT_AVATAR_URL: &str = + "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png"; + +pub(crate) const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ + KnownAcpRuntime { + id: "goose", + label: "Goose", + commands: &["goose"], + aliases: &[], + avatar_url: GOOSE_AVATAR_URL, + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("goose"), + cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], + // Goose's stable release currently publishes only the Unix installer; + // its official Windows instructions intentionally point at this main-branch script. + cli_install_commands_windows: &[windows_install_command!("goose", "https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1", "$env:CONFIGURE='false'; ")], + adapter_install_commands: &[], + cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz talks to Goose through the Goose CLI.", + adapter_install_hint: "", + skill_dir: Some(".goose/skills"), + supports_acp_model_switching: false, + model_env_var: Some("GOOSE_MODEL"), + provider_env_var: Some("GOOSE_PROVIDER"), + provider_locked: false, + default_env: &[("GOOSE_MODE", "auto")], + config_file_path: Some("~/.config/goose/config.yaml"), + config_file_format: Some("yaml"), + supports_acp_native_config: true, + thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + thinking_config_json_env_var: None, + thinking_config_json_key: None, + max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), + context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + max_rounds_env_var: None, + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + }, + KnownAcpRuntime { + id: "claude", + label: "Claude Code", + commands: &["claude-agent-acp", "claude-code-acp"], + aliases: &["claude-code", "claudecode"], + avatar_url: CLAUDE_CODE_AVATAR_URL, + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("claude"), + cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], + cli_install_commands_windows: &[windows_install_command!("claude", "https://claude.ai/install.ps1")], + adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], + cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", + cli_install_hint: "Buzz talks to Claude Code through the Claude Code CLI.", + adapter_install_hint: "Buzz talks to the Claude Code CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/claude-agent-acp.", + skill_dir: Some(".claude/skills"), + supports_acp_model_switching: false, + model_env_var: None, + provider_env_var: None, + provider_locked: true, + default_env: &[], + config_file_path: Some("~/.claude/settings.json"), + config_file_format: Some("json"), + supports_acp_native_config: false, + thinking_env_var: Some("CLAUDE_CODE_EFFORT_LEVEL"), + thinking_config_json_env_var: None, + thinking_config_json_key: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run the Claude CLI to complete authentication."), + auth_probe_args: Some(&["claude", "auth", "status"]), + }, + KnownAcpRuntime { + id: "codex", + label: "Codex", + commands: &["codex-acp"], + aliases: &[], + avatar_url: CODEX_AVATAR_URL, + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: false, + underlying_cli: Some("codex"), + cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], + cli_install_commands_windows: &[windows_install_command!("codex", "https://chatgpt.com/codex/install.ps1")], + adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], + cli_install_instructions_url: "https://developers.openai.com/codex/cli/", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", + cli_install_hint: "Buzz talks to Codex through the Codex CLI.", + adapter_install_hint: "Buzz talks to the Codex CLI through an ACP adapter. Install it with: npm install -g @agentclientprotocol/codex-acp.", + skill_dir: Some(".codex/skills"), + supports_acp_model_switching: false, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + config_file_path: Some("~/.codex/config.toml"), + config_file_format: Some("toml"), + supports_acp_native_config: false, + thinking_env_var: None, + thinking_config_json_env_var: Some("CODEX_CONFIG"), + thinking_config_json_key: Some("model_reasoning_effort"), + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run `codex login` to authenticate."), + // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. + auth_probe_args: Some(&["codex", "login", "status"]), + }, + KnownAcpRuntime { + id: "buzz-agent", + label: "Buzz Agent", + commands: &["buzz-agent"], + aliases: &[], + avatar_url: BUZZ_AGENT_AVATAR_URL, + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: true, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "https://github.com/block/buzz", + adapter_install_instructions_url: "https://github.com/block/buzz", + cli_install_hint: "Ships with the Buzz desktop app.", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: true, + model_env_var: Some("BUZZ_AGENT_MODEL"), + provider_env_var: Some("BUZZ_AGENT_PROVIDER"), + provider_locked: false, + default_env: &[], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), + thinking_config_json_env_var: None, + thinking_config_json_key: None, + max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), + context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + max_rounds_env_var: Some("BUZZ_AGENT_MAX_ROUNDS"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + }, +]; diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index d86e5f33f0..8c6c7d4436 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -67,6 +67,8 @@ pub(super) fn preset_catalog_entry( model_env_var: None, provider_env_var: None, thinking_env_var: None, + thinking_config_json_env_var: None, + thinking_config_json_key: None, max_tokens_env_var: None, context_limit_env_var: None, max_rounds_env_var: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs index 34edecdcd9..cf962c5779 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs @@ -47,6 +47,11 @@ pub(crate) struct KnownAcpRuntime { pub config_file_format: Option<&'static str>, pub supports_acp_native_config: bool, // tier 1a: config/read+write pub thinking_env_var: Option<&'static str>, + /// Structured JSON environment configuration target for thinking effort. + /// The value is a JSON object; `thinking_config_json_key` identifies the + /// scalar property that carries the selected effort level. + pub thinking_config_json_env_var: Option<&'static str>, + pub thinking_config_json_key: Option<&'static str>, /// Env var for normalizing `max_output_tokens`. `None` when the harness /// does not have a first-class env var for this field (config-file only). pub max_tokens_env_var: Option<&'static str>, diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 9ca5fd080d..01e9521614 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -185,6 +185,8 @@ pub(crate) fn is_safe_to_reveal(key: &str) -> bool { "BUZZ_AGENT_PROVIDER", "BUZZ_AGENT_MODEL", "BUZZ_AGENT_THINKING_EFFORT", + "CLAUDE_CODE_EFFORT_LEVEL", + "GOOSE_THINKING_EFFORT", "DATABRICKS_HOST", "DATABRICKS_MODEL", ]; @@ -192,6 +194,104 @@ pub(crate) fn is_safe_to_reveal(key: &str) -> bool { SAFE_KEYS.iter().any(|safe| upper == *safe) } +/// Project only explicitly approved, non-secret environment settings for a +/// portable agent/team snapshot. This is intentionally separate from the UI +/// display policy's callers so export never accidentally serializes raw env. +pub(crate) fn portable_env_for_export( + env_vars: &BTreeMap, +) -> BTreeMap { + env_vars + .iter() + .filter(|(key, _)| is_safe_to_reveal(key)) + .map(|(key, value)| (key.clone(), value.clone())) + .collect() +} + +/// Accept the portable snapshot projection only after applying the same +/// allowlist and normal save-time environment validation used by UI input. +/// Unsafe crafted keys are dropped rather than becoming configuration. +pub(crate) fn portable_env_for_import( + portable_env: &BTreeMap, +) -> Result, String> { + let projected = portable_env_for_export(portable_env); + validate_user_env_keys(&projected)?; + Ok(projected) +} + +/// Extract a portable thinking value from the runtime-specific persisted +/// representation. Plain environment targets store a level directly; structured +/// JSON targets (currently Codex) store it at their catalog-declared property. +pub(crate) fn thinking_effort_from_record( + record: &crate::managed_agents::types::ManagedAgentRecord, +) -> Option { + let runtime = record + .runtime + .as_deref() + .and_then(crate::managed_agents::known_acp_runtime_exact) + .or_else(|| crate::managed_agents::known_acp_runtime(&record.agent_command))?; + + if let (Some(env_key), Some(json_key)) = ( + runtime.thinking_config_json_env_var, + runtime.thinking_config_json_key, + ) { + return json_env_string(&record.env_vars, env_key, json_key); + } + + runtime + .thinking_env_var + .and_then(|key| record.env_vars.get(key)) + .cloned() +} + +/// Restore the safe, portable portion of a snapshot's runtime configuration. +/// A first-class effort value is written to the catalog-declared target: a +/// direct env var for standard harnesses, or a minimal JSON object for Codex. +pub(crate) fn portable_config_for_import( + runtime_id: Option<&str>, + portable_env: &BTreeMap, + thinking_effort: Option<&str>, +) -> Result, String> { + let mut env_vars = portable_env_for_import(portable_env)?; + let Some(runtime) = runtime_id.and_then(crate::managed_agents::known_acp_runtime_exact) else { + return Ok(env_vars); + }; + let Some(effort) = thinking_effort.filter(|value| !value.is_empty()) else { + return Ok(env_vars); + }; + + if let (Some(env_key), Some(json_key)) = ( + runtime.thinking_config_json_env_var, + runtime.thinking_config_json_key, + ) { + env_vars.insert( + env_key.to_string(), + serde_json::json!({ json_key: effort }).to_string(), + ); + } else if let Some(env_key) = runtime.thinking_env_var { + // The first-class snapshot field is authoritative when the two values + // differ, while the allowlisted env map remains available for all other + // non-secret runtime configuration. + env_vars.insert(env_key.to_string(), effort.to_string()); + } + Ok(env_vars) +} + +fn json_env_string( + env_vars: &BTreeMap, + env_key: &str, + json_key: &str, +) -> Option { + env_vars + .get(env_key) + .and_then(|raw| serde_json::from_str::(raw).ok()) + .and_then(|value| { + value + .get(json_key) + .and_then(serde_json::Value::as_str) + .map(str::to_string) + }) +} + /// Per-value byte cap for env values. 32 KiB is generous for credentials, /// JWT-ish tokens, certs etc., but small enough that a malformed IPC /// caller can't blow up the persona/agent JSON file. Tune up if real diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index 34cdfede2c..11e49d58ea 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -2,8 +2,9 @@ use std::collections::BTreeMap; use super::{ display_invalid_key, is_derived_provider_model_key, is_reserved_env_key, - is_well_formed_env_key, merged_user_env, validate_user_env_keys, - DERIVED_PROVIDER_MODEL_ENV_KEYS, MAX_ENV_TOTAL_BYTES, MAX_ENV_VALUE_BYTES, RESERVED_ENV_KEYS, + is_well_formed_env_key, json_env_string, merged_user_env, portable_config_for_import, + portable_env_for_import, validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS, + MAX_ENV_TOTAL_BYTES, MAX_ENV_VALUE_BYTES, RESERVED_ENV_KEYS, }; fn map(pairs: &[(&str, &str)]) -> BTreeMap { @@ -501,3 +502,57 @@ fn deploy_model_precedence_none_when_both_absent() { let effective = persona_model.clone().or(record_model.clone()); assert_eq!(effective, None); } + +// ── portable snapshot configuration ───────────────────────────────── + +#[test] +fn portable_import_restores_codex_effort_and_drops_crafted_secrets() { + let source = map(&[ + ("CLAUDE_CODE_EFFORT_LEVEL", "medium"), + ("OPENAI_API_KEY", "secret"), + ("UNRELATED_SETTING", "not-portable"), + ]); + + let env_vars = portable_config_for_import(Some("codex"), &source, Some("high")).unwrap(); + + assert_eq!( + env_vars.get("CLAUDE_CODE_EFFORT_LEVEL").map(String::as_str), + Some("medium") + ); + assert!(!env_vars.contains_key("OPENAI_API_KEY")); + assert!(!env_vars.contains_key("UNRELATED_SETTING")); + assert_eq!( + env_vars.get("CODEX_CONFIG").map(String::as_str), + Some(r#"{"model_reasoning_effort":"high"}"#) + ); +} + +#[test] +fn codex_effort_reader_ignores_malformed_or_non_string_json_values() { + let malformed = map(&[("CODEX_CONFIG", "not-json")]); + assert_eq!( + json_env_string(&malformed, "CODEX_CONFIG", "model_reasoning_effort"), + None + ); + + let non_string = map(&[("CODEX_CONFIG", r#"{"model_reasoning_effort":3}"#)]); + assert_eq!( + json_env_string(&non_string, "CODEX_CONFIG", "model_reasoning_effort"), + None + ); +} + +#[test] +fn portable_import_drops_crafted_secret_without_persisting_it() { + let imported = portable_env_for_import(&map(&[ + ("GOOSE_THINKING_EFFORT", "high"), + ("ANTHROPIC_API_KEY", "sk-ant-secret"), + ])) + .unwrap(); + + assert_eq!( + imported.get("GOOSE_THINKING_EFFORT").map(String::as_str), + Some("high") + ); + assert!(!imported.contains_key("ANTHROPIC_API_KEY")); +} diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c072448ff1..2926126cdc 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -659,6 +659,8 @@ mod tests { use super::*; use crate::managed_agents::discovery::known_acp_runtime_exact; + use super::runtime_fixtures::{make_cli_runtime, make_codex_runtime}; + /// Build a minimal `EffectiveAgentEnv` with the given env map and command. fn make_env(command: &str, env: BTreeMap) -> EffectiveAgentEnv { let runtime = known_acp_runtime_exact(command); @@ -1017,47 +1019,6 @@ mod tests { // ── cli_login_requirements: resolve_command integration ───────────── - /// Construct a minimal `KnownAcpRuntime` stub for testing cli_login_requirements. - /// `commands` are the adapter binaries; `underlying_cli` is the CLI name. - fn make_cli_runtime( - commands: &'static [&'static str], - underlying_cli: Option<&'static str>, - ) -> KnownAcpRuntime { - KnownAcpRuntime { - id: "test-cli-runtime", - label: "Test CLI", - commands, - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, - underlying_cli, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - config_file_path: None, - config_file_format: None, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, - } - } - /// Returns the absolute path of the currently-running test binary as a `&'static str`. /// Host-portable stand-in for a "present" binary: absolute path so `find_command` resolves /// it via `path.exists()`. Leaked allocation is intentional — process exits after tests. @@ -1206,50 +1167,6 @@ mod tests { // ── codex readiness version gate ─────────────────────────────────────── - /// Build a minimal `KnownAcpRuntime` for testing the codex version gate. - /// `adapter_commands` are the exact strings passed to `find_command` — use - /// `&["codex-acp"]` when the binary is on PATH, or `&[]` - /// when resolving via absolute path. `underlying_cli` is a portable - /// stand-in so the adapter is not misclassified as `CliMissing`. - fn make_codex_runtime( - adapter_commands: &'static [&'static str], - underlying_cli: Option<&'static str>, - ) -> KnownAcpRuntime { - KnownAcpRuntime { - id: "codex", - label: "Codex", - commands: adapter_commands, - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, - underlying_cli, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - config_file_path: None, - config_file_format: None, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, - } - } - /// Build a temp dir containing a `codex-acp` script with the given body, /// prepend it to PATH, and clear the resolve cache. Returns the temp dir /// and the original PATH string for restoration. @@ -1734,6 +1651,12 @@ mod tests { } } +// `KnownAcpRuntime` fixtures for the tests above live in a sibling file so this +// module stays under the desktop file-size ratchet. +#[cfg(test)] +#[path = "readiness_test_runtimes.rs"] +mod runtime_fixtures; + // Goose file-config-aware requirement tests live in a sibling file so this // module stays under the desktop file-size ratchet. #[cfg(test)] diff --git a/desktop/src-tauri/src/managed_agents/readiness_test_runtimes.rs b/desktop/src-tauri/src/managed_agents/readiness_test_runtimes.rs new file mode 100644 index 0000000000..2f5e6fd767 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness_test_runtimes.rs @@ -0,0 +1,96 @@ +//! `KnownAcpRuntime` fixtures for the `readiness.rs` unit tests, split out of +//! that file to respect the repo's file-size ratchet. +//! +//! Declared as `mod runtime_fixtures` from `readiness.rs`, so `use super::*` +//! reaches the types that module already imports. + +use super::*; + +/// Construct a minimal `KnownAcpRuntime` stub for testing cli_login_requirements. +/// `commands` are the adapter binaries; `underlying_cli` is the CLI name. +pub(super) fn make_cli_runtime( + commands: &'static [&'static str], + underlying_cli: Option<&'static str>, +) -> KnownAcpRuntime { + KnownAcpRuntime { + id: "test-cli-runtime", + label: "Test CLI", + commands, + aliases: &[], + avatar_url: "", + mcp_command: None, + mcp_hooks: false, + underlying_cli, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "", + adapter_install_instructions_url: "", + cli_install_hint: "", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + config_file_path: None, + config_file_format: None, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + supports_acp_native_config: false, + thinking_env_var: None, + thinking_config_json_env_var: None, + thinking_config_json_key: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + } +} + +/// Build a minimal `KnownAcpRuntime` for testing the codex version gate. +/// `adapter_commands` are the exact strings passed to `find_command` — use +/// `&["codex-acp"]` when the binary is on PATH, or `&[]` +/// when resolving via absolute path. `underlying_cli` is a portable +/// stand-in so the adapter is not misclassified as `CliMissing`. +pub(super) fn make_codex_runtime( + adapter_commands: &'static [&'static str], + underlying_cli: Option<&'static str>, +) -> KnownAcpRuntime { + KnownAcpRuntime { + id: "codex", + label: "Codex", + commands: adapter_commands, + aliases: &[], + avatar_url: "", + mcp_command: None, + mcp_hooks: false, + underlying_cli, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "", + adapter_install_instructions_url: "", + cli_install_hint: "", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + config_file_path: None, + config_file_format: None, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + supports_acp_native_config: false, + thinking_env_var: None, + thinking_config_json_env_var: None, + thinking_config_json_key: None, + max_tokens_env_var: None, + context_limit_env_var: None, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + } +} diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index e5be105fed..cca4e97cc5 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -626,53 +626,6 @@ pub enum HarnessSource { Custom, } -#[derive(Debug, Clone, Serialize)] -pub struct AcpRuntimeCatalogEntry { - pub id: String, - pub label: String, - pub avatar_url: String, - pub availability: AcpAvailabilityStatus, - pub command: Option, - pub binary_path: Option, - pub default_args: Vec, - pub mcp_command: Option, - /// Environment variable used to apply the initial model, when supported. - pub model_env_var: Option, - /// Environment variable used to apply the selected LLM provider, when supported. - pub provider_env_var: Option, - /// Environment variable used to apply thinking effort, when supported. - pub thinking_env_var: Option, - pub max_tokens_env_var: Option, - pub context_limit_env_var: Option, - pub max_rounds_env_var: Option, - pub install_hint: String, - pub install_instructions_url: String, - /// true when at least one automated install step is available - pub can_auto_install: bool, - /// true when this runtime depends on a separately installed vendor CLI. - pub requires_external_cli: bool, - pub underlying_cli_path: Option, - /// true when an npm adapter step is pending but Node.js / npm is absent. - /// The UI hides the Install button and shows a Node.js install callout. - pub node_required: bool, - /// Login/authentication status for CLI-based runtimes. - pub auth_status: AuthStatus, - /// Hint for completing authentication, shown when `auth_status` is not `logged_in`. - #[serde(skip_serializing_if = "Option::is_none")] - pub login_hint: Option, - /// Whether this entry came from the compiled-in catalog or a user-supplied - /// JSON file in `custom_harnesses/`. The UI uses this to decide editability. - pub source: HarnessSource, - /// Definition-level env vars for `source: custom` entries; populated from - /// `HarnessDefinition.env` so saves don't silently erase existing vars. - /// Absent for builtin/preset entries. Skipped when empty in serialization. - #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub definition_env: BTreeMap, - /// Spawn-time parallelism cap; absent for uncapped harnesses. - #[serde(skip_serializing_if = "Option::is_none")] - pub max_parallelism: Option, -} - /// Result of a single install step (CLI or adapter). #[derive(Debug, Clone, Serialize)] pub struct InstallStepResult { @@ -990,6 +943,8 @@ pub fn resolve_mint_behavioral_defaults( mod catalog_source; pub use catalog_source::CatalogSource; +mod runtime_catalog; +pub use runtime_catalog::AcpRuntimeCatalogEntry; mod requests; pub use requests::*; diff --git a/desktop/src-tauri/src/managed_agents/types/runtime_catalog.rs b/desktop/src-tauri/src/managed_agents/types/runtime_catalog.rs new file mode 100644 index 0000000000..a43aeb4a3f --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/types/runtime_catalog.rs @@ -0,0 +1,58 @@ +//! The ACP runtime catalog entry surfaced to the UI, split from `types.rs` +//! (file-size cap). + +use serde::Serialize; +use std::collections::BTreeMap; + +use super::{AcpAvailabilityStatus, AuthStatus, HarnessSource}; + +#[derive(Debug, Clone, Serialize)] +pub struct AcpRuntimeCatalogEntry { + pub id: String, + pub label: String, + pub avatar_url: String, + pub availability: AcpAvailabilityStatus, + pub command: Option, + pub binary_path: Option, + pub default_args: Vec, + pub mcp_command: Option, + /// Environment variable used to apply the initial model, when supported. + pub model_env_var: Option, + /// Environment variable used to apply the selected LLM provider, when supported. + pub provider_env_var: Option, + /// Environment variable used to apply thinking effort, when supported. + pub thinking_env_var: Option, + /// Structured JSON env-var target for thinking effort, when supported. + pub thinking_config_json_env_var: Option, + /// JSON property in `thinking_config_json_env_var` holding the effort value. + pub thinking_config_json_key: Option, + pub max_tokens_env_var: Option, + pub context_limit_env_var: Option, + pub max_rounds_env_var: Option, + pub install_hint: String, + pub install_instructions_url: String, + /// true when at least one automated install step is available + pub can_auto_install: bool, + /// true when this runtime depends on a separately installed vendor CLI. + pub requires_external_cli: bool, + pub underlying_cli_path: Option, + /// true when an npm adapter step is pending but Node.js / npm is absent. + /// The UI hides the Install button and shows a Node.js install callout. + pub node_required: bool, + /// Login/authentication status for CLI-based runtimes. + pub auth_status: AuthStatus, + /// Hint for completing authentication, shown when `auth_status` is not `logged_in`. + #[serde(skip_serializing_if = "Option::is_none")] + pub login_hint: Option, + /// Whether this entry came from the compiled-in catalog or a user-supplied + /// JSON file in `custom_harnesses/`. The UI uses this to decide editability. + pub source: HarnessSource, + /// Definition-level env vars for `source: custom` entries; populated from + /// `HarnessDefinition.env` so saves don't silently erase existing vars. + /// Absent for builtin/preset entries. Skipped when empty in serialization. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub definition_env: BTreeMap, + /// Spawn-time parallelism cap; absent for uncapped harnesses. + #[serde(skip_serializing_if = "Option::is_none")] + pub max_parallelism: Option, +} diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index b929dbb613..ef6746b753 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -43,12 +43,14 @@ with a TypeScript lookup table or an id comparison in a component. in a component. Components ask the field model what exists (`hasRenderableAgentConfigField`, `getRenderableEffortField`). 2. **Effort reads/writes go through the descriptor.** Use the effort - descriptor's `currentPersistence` key — never a raw - `BUZZ_AGENT_THINKING_EFFORT` literal in UI code. `currentPersistence` is - where the value lives *today*; `targetApplication` is how the harness - *should* receive it. They intentionally differ until PR 2.7 migrates - Goose/Claude — do not "fix" one to match the other without doing the - migration work. + descriptor's `currentPersistence` target — never a raw + `BUZZ_AGENT_THINKING_EFFORT` literal in UI code. A descriptor may own a + direct environment variable, an ACP option, or one field inside a structured + JSON environment value; use its target-specific helpers so unrelated JSON + configuration survives edits. `currentPersistence` is where the value lives + *today*; `targetApplication` is how the harness *should* receive it. They + intentionally differ until PR 2.7 migrates Goose/Claude — do not "fix" one + to match the other without doing the migration work. 3. **Field absence has a named reason, not a boolean.** Codex effort is `ownedByModelId`; Claude effort is `deferredUntilNativeOptionsAvailable`. New absences get new named reasons in `AgentConfigOmission` / diff --git a/desktop/src/features/agents/lib/agentConfigCore.test.mjs b/desktop/src/features/agents/lib/agentConfigCore.test.mjs index 92159ff275..ac195578bc 100644 --- a/desktop/src/features/agents/lib/agentConfigCore.test.mjs +++ b/desktop/src/features/agents/lib/agentConfigCore.test.mjs @@ -4,7 +4,10 @@ import test from "node:test"; import { deriveAgentConfigFieldModel, deriveNumericDescriptors, + deriveStructuredJsonEffortDescriptor, + readStructuredJsonEnvValue, structuredEnvKeys, + updateStructuredJsonEnvValue, } from "./agentConfigCore.ts"; import { NUMERIC_KIND_MIN } from "../ui/buzzAgentModelTuningFields.tsx"; @@ -28,6 +31,8 @@ function runtime(id, metadata = {}) { modelEnvVar: null, providerEnvVar: null, thinkingEnvVar: null, + thinkingConfigJsonEnvVar: null, + thinkingConfigJsonKey: null, maxTokensEnvVar: null, contextLimitEnvVar: null, maxRoundsEnvVar: null, @@ -46,6 +51,65 @@ function field(model, kind) { return model.fields.find((candidate) => candidate.kind === kind); } +test("structured JSON effort preserves unrelated runtime configuration", () => { + const descriptor = deriveStructuredJsonEffortDescriptor( + runtime("codex", { + thinkingConfigJsonEnvVar: "CODEX_CONFIG", + thinkingConfigJsonKey: "model_reasoning_effort", + }), + ); + assert.ok(descriptor); + + const initial = { + CODEX_CONFIG: JSON.stringify({ + other_setting: "keep", + model_reasoning_effort: "medium", + }), + }; + assert.equal( + readStructuredJsonEnvValue(initial, descriptor.currentPersistence), + "medium", + ); + const updated = updateStructuredJsonEnvValue( + initial, + descriptor.currentPersistence, + "high", + ); + assert.deepEqual(JSON.parse(updated.CODEX_CONFIG), { + other_setting: "keep", + model_reasoning_effort: "high", + }); + const cleared = updateStructuredJsonEnvValue( + updated, + descriptor.currentPersistence, + "", + ); + assert.deepEqual(JSON.parse(cleared.CODEX_CONFIG), { other_setting: "keep" }); +}); + +test("structured JSON effort replaces malformed JSON only when edited", () => { + const descriptor = deriveStructuredJsonEffortDescriptor( + runtime("codex", { + thinkingConfigJsonEnvVar: "CODEX_CONFIG", + thinkingConfigJsonKey: "model_reasoning_effort", + }), + ); + assert.ok(descriptor); + const initial = { CODEX_CONFIG: "not-json" }; + assert.equal( + readStructuredJsonEnvValue(initial, descriptor.currentPersistence), + "", + ); + const updated = updateStructuredJsonEnvValue( + initial, + descriptor.currentPersistence, + "high", + ); + assert.deepEqual(JSON.parse(updated.CODEX_CONFIG), { + model_reasoning_effort: "high", + }); +}); + test("Buzz Agent exposes provider, model, and Buzz-owned effort", () => { const model = deriveAgentConfigFieldModel({ config, diff --git a/desktop/src/features/agents/lib/agentConfigCore.ts b/desktop/src/features/agents/lib/agentConfigCore.ts index 5a8b8cb1c3..aae04396ff 100644 --- a/desktop/src/features/agents/lib/agentConfigCore.ts +++ b/desktop/src/features/agents/lib/agentConfigCore.ts @@ -2,7 +2,10 @@ import type { AcpRuntimeCatalogEntry, GlobalAgentConfig, } from "@/shared/api/types"; -import { BUZZ_AGENT_THINKING_EFFORT } from "../ui/buzzAgentConfig"; +import { + BUZZ_AGENT_THINKING_EFFORT, + BUZZ_AGENT_THINKING_EFFORT_VALUES, +} from "../ui/buzzAgentConfig"; /** * Lifecycle status of the ACP runtime catalog query on a per-agent surface. @@ -37,6 +40,12 @@ type EnvVarPersistence = { key: string; }; +export type StructuredJsonEnvPersistence = { + kind: "structuredJsonEnv"; + envKey: string; + jsonKey: string; +}; + type AcpConfigOptionPersistence = { kind: "acpConfigOption"; id: string; @@ -74,10 +83,12 @@ export type AgentConfigFieldDescriptor = | "harnessNative"; currentPersistence: | EnvVarPersistence + | StructuredJsonEnvPersistence | AcpConfigOptionPersistence | UnavailablePersistence; targetApplication: | { kind: "envVar"; key: string } + | { kind: "structuredJsonEnv"; envKey: string; jsonKey: string } | { kind: "acpConfigOption"; id: string; category: string }; render: "control" | "deferredUntilNativeOptionsAvailable"; value: string | null; @@ -117,6 +128,109 @@ function valueFromEnv(config: GlobalAgentConfig, key: string) { return config.env_vars[key]?.trim() || null; } +export type StructuredJsonEffortDescriptor = { + kind: "effort"; + optionSource: "legacyProviderModelCatalog"; + currentPersistence: StructuredJsonEnvPersistence; + targetApplication: { + kind: "structuredJsonEnv"; + envKey: string; + jsonKey: string; + }; + render: "control"; + value: null; + values: readonly string[]; +}; + +/** + * Derives an effort field for a catalog-declared structured JSON env target. + * The renderer never needs to know a runtime ID, env name, or JSON property. + */ +export function deriveStructuredJsonEffortDescriptor( + runtime: AcpRuntimeCatalogEntry | undefined, +): StructuredJsonEffortDescriptor | undefined { + if (!runtime?.thinkingConfigJsonEnvVar || !runtime.thinkingConfigJsonKey) { + return undefined; + } + return { + kind: "effort", + optionSource: "legacyProviderModelCatalog", + currentPersistence: { + kind: "structuredJsonEnv", + envKey: runtime.thinkingConfigJsonEnvVar, + jsonKey: runtime.thinkingConfigJsonKey, + }, + targetApplication: { + kind: "structuredJsonEnv", + envKey: runtime.thinkingConfigJsonEnvVar, + jsonKey: runtime.thinkingConfigJsonKey, + }, + render: "control", + value: null, + values: BUZZ_AGENT_THINKING_EFFORT_VALUES, + }; +} + +/** Read one string property from a descriptor-owned JSON env value. */ +export function readStructuredJsonEnvValue( + envVars: Record, + persistence: StructuredJsonEnvPersistence, +): string { + try { + const parsed: unknown = JSON.parse(envVars[persistence.envKey] ?? "{}"); + if ( + parsed !== null && + typeof parsed === "object" && + typeof (parsed as Record)[persistence.jsonKey] === + "string" + ) { + return (parsed as Record)[persistence.jsonKey]; + } + } catch { + // Generic env editing may have left invalid JSON; preserve it until the + // user selects an effort rather than hiding or silently discarding it. + } + return ""; +} + +/** + * Update one descriptor-owned JSON property without discarding unrelated + * configuration. Blank removes only that property, deleting the env key only + * when it becomes an empty object. + */ +export function updateStructuredJsonEnvValue( + envVars: Record, + persistence: StructuredJsonEnvPersistence, + value: string, +): Record { + let config: Record = {}; + try { + const parsed: unknown = JSON.parse(envVars[persistence.envKey] ?? "{}"); + if ( + parsed !== null && + typeof parsed === "object" && + !Array.isArray(parsed) + ) { + config = { ...(parsed as Record) }; + } + } catch { + // Selecting a structured value replaces invalid JSON with the minimal + // valid object required by this target. + } + if (value) { + config[persistence.jsonKey] = value; + } else { + delete config[persistence.jsonKey]; + } + const next = { ...envVars }; + if (Object.keys(config).length === 0) { + delete next[persistence.envKey]; + } else { + next[persistence.envKey] = JSON.stringify(config); + } + return next; +} + /** * Derives the numeric descriptor set for a runtime from catalog fields. * @@ -301,8 +415,12 @@ export function structuredEnvKeys( const keys: string[] = []; for (const d of renderedDescriptors) { if (d.render !== "control") continue; - if (d.kind === "effort" && d.currentPersistence.kind === "envVar") { - keys.push(d.currentPersistence.key); + if (d.kind === "effort") { + if (d.currentPersistence.kind === "envVar") { + keys.push(d.currentPersistence.key); + } else if (d.currentPersistence.kind === "structuredJsonEnv") { + keys.push(d.currentPersistence.envKey); + } } else if ( d.kind === "maxOutputTokens" || d.kind === "contextLimit" || diff --git a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx index 6b42b10497..6487e99a3d 100644 --- a/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx +++ b/desktop/src/features/agents/ui/EditAgentAdvancedFields.tsx @@ -9,8 +9,7 @@ import { PERSONA_FIELD_SHELL_CLASS, PERSONA_LABEL_OPTIONAL_CLASS, } from "./agentConfigOptions"; -import type { AgentPersona } from "@/shared/api/types"; -import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; +import type { AgentPersona, AcpRuntimeCatalogEntry } from "@/shared/api/types"; import { BuzzAgentModelTuningFields, NumericTuningFields, @@ -24,8 +23,11 @@ import { parallelismCapHint, } from "../lib/agentParallelism"; import { + deriveStructuredJsonEffortDescriptor, deriveNumericDescriptors, + readStructuredJsonEnvValue, structuredEnvKeys, + updateStructuredJsonEnvValue, type RuntimeCatalogStatus, } from "../lib/agentConfigCore"; @@ -118,8 +120,22 @@ export function EditAgentAdvancedFields({ [catalogStatus, selectedRuntime], ); - // Build the effective hidden-key list: caller's secrets + effort key (when - // rendered by BuzzAgentModelTuningFields) + numeric keys via structuredEnvKeys. + const structuredEffortDescriptor = React.useMemo( + () => + catalogStatus === "ready" + ? deriveStructuredJsonEffortDescriptor(selectedRuntime) + : undefined, + [catalogStatus, selectedRuntime], + ); + const structuredEffort = structuredEffortDescriptor + ? readStructuredJsonEnvValue( + envVars, + structuredEffortDescriptor.currentPersistence, + ) + : ""; + + // A structured JSON target has a dedicated selector, so hide its raw env + // row only when the catalog is ready and that selector actually renders. const effectiveHiddenKeys = React.useMemo( () => [ ...hiddenEnvKeys, @@ -127,8 +143,16 @@ export function EditAgentAdvancedFields({ ? [BUZZ_AGENT_THINKING_EFFORT] : []), ...structuredEnvKeys(numericDescriptors), + ...structuredEnvKeys( + structuredEffortDescriptor ? [structuredEffortDescriptor] : [], + ), + ], + [ + hiddenEnvKeys, + modelTuningRuntimeId, + numericDescriptors, + structuredEffortDescriptor, ], - [hiddenEnvKeys, modelTuningRuntimeId, numericDescriptors], ); // Harness cap hint: show only when the selected runtime has a cap and the @@ -195,6 +219,43 @@ export function EditAgentAdvancedFields({

+ {/* Catalog-backed structured JSON effort target. */} + {structuredEffortDescriptor ? ( +
+ + +

+ Applied through this runtime's structured configuration. +

+
+ ) : null} + {/* Agent runtime args */}