diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index bb9e2e581f..f4b68c8fc0 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -526,6 +526,10 @@ mod tests { is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, agent_command_override: None, persona_source_version: None, @@ -548,6 +552,10 @@ mod tests { source_team: None, source_team_persona_slug: None, env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "".to_string(), updated_at: "".to_string(), } diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 7282e3cc66..565e0fcc10 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -301,104 +301,6 @@ async fn start_local_agent_with_preflight( build_managed_agent_summary(app, record, &runtimes, &personas) } -/// Build the standard agent JSON payload for provider deploy calls. -/// -/// Like local spawn, provider deploy re-reads live persona env vars and -/// structured model/provider so remote agents receive current credentials -/// and the same authoritative values that local spawn derives from -/// `runtime_metadata_env_vars`. The only field still pinned is -/// `agent_command`/`agent_args` — those were captured at create time. -/// The only read-time resolution is `relay_url`: a blank pin resolves to -/// the active workspace relay here, matching the create-path contract. -/// -/// Fails closed when the private key is unavailable (keyring outage leaves -/// it empty after hydration): without this guard a provider deploy would -/// serialize `"private_key_nsec": ""` and launch the agent with no -/// identity — the same hazard the local spawn path refuses via -/// `spawn_key_refusal`. -fn build_deploy_payload( - app: &AppHandle, - state: &AppState, - record: &ManagedAgentRecord, -) -> Result { - // Fails closed when the private key is unavailable — same guard as local - // spawn. Without this, a keyring outage would serialize `"private_key_nsec": ""` - // and launch the agent with no identity. - if let Some(err) = crate::managed_agents::spawn_key_refusal(record) { - return Err(err); - } - - // Merge persona env_vars + agent env_vars for provider deploy — the same - // live-persona-under-overrides semantics as local spawn. Without this, - // provider-backed agents wouldn't receive credentials saved on the persona - // or the agent itself. - let persona_env = - crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?; - let merged_env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars); - - // Resolve the persona's structured provider/model so the remote provider - // receives the same authoritative values that local spawn derives from - // `runtime_metadata_env_vars`. Without this, remote deploy would rely on - // stale derived env copies in `env_vars` (or have no provider at all for - // imported personas whose derived keys were filtered at import time). - // - // Precedence: persona field wins when non-blank; otherwise falls back to the - // record's own field (same blank-normalization as the snapshot path). This - // matches `persona_snapshot_with_agent_config_fallback` exactly — a blank - // persona field must not wipe a record value that the user configured. - let (effective_model, effective_provider) = if let Some(pid) = record.persona_id.as_deref() { - let personas = load_personas(app).map_err(|e| { - format!( - "failed to load personas while building deploy payload for persona `{pid}`: {e}" - ) - })?; - let persona = personas - .into_iter() - .find(|p| p.id == pid) - .ok_or_else(|| format!("persona `{pid}` not found while building deploy payload"))?; - let fallback = crate::managed_agents::persona_events::persona_field_with_record_fallback; - let model = fallback(persona.model.as_deref(), record.model.as_deref()); // fallback: record.model - let provider = fallback(persona.provider.as_deref(), record.provider.as_deref()); // fallback: record.provider - (model, provider) - } else { - (record.model.clone(), record.provider.clone()) - }; - - Ok(serde_json::json!({ - "name": &record.name, - // Resolve the per-agent pin against the active workspace relay here: - // this payload crosses the host boundary to a remote provider harness - // that has no notion of the desktop's workspace, so the blank→workspace - // fallback (otherwise applied at read-time in `effective_agent_relay_url`) - // must be materialized into a concrete URL before serializing. - "relay_url": crate::relay::effective_agent_relay_url( - &record.relay_url, - &relay_ws_url_with_override(state), - ), - "private_key_nsec": &record.private_key_nsec, - "auth_tag": &record.auth_tag, - "agent_command": &record.agent_command, - "agent_args": &record.agent_args, - "system_prompt": &record.system_prompt, - "model": effective_model, - // Structured provider from the persona record. Providers that don't - // yet read this field will fall back to env_vars or their own default - // — no protocol break. - "provider": effective_provider, - "turn_timeout_seconds": record.turn_timeout_seconds, - "idle_timeout_seconds": record.idle_timeout_seconds, - "max_turn_duration_seconds": record.max_turn_duration_seconds, - "parallelism": record.parallelism, - // Inbound author gate. Providers that don't yet read these fall back - // to the harness default (`owner-only`) — no protocol break. - "respond_to": record.respond_to, - "respond_to_allowlist": &record.respond_to_allowlist, - // Merged persona + agent env vars. Providers that don't read this - // field will simply ignore it — no protocol break. - "env_vars": merged_env, - })) -} - /// Deploy an agent to a provider backend. Resolves the binary, calls deploy via /// spawn_blocking, and persists the result (backend_agent_id or last_error). /// @@ -532,10 +434,13 @@ pub async fn create_managed_agent( // Validate & normalize the respond-to allowlist BEFORE any side effects. // The harness has its own validator (buzz-acp/src/config.rs) but we want // to catch malformed input at the boundary so the agent never tries to - // start with a list that will crash it on launch. + // start with a list that will crash it on launch. The mode/allowlist + // pairing (and the definition-default fallback) is resolved later at the + // mint site via `resolve_mint_behavioral_defaults`, where the linked + // definition is in hand. let respond_to_allowlist = crate::managed_agents::validate_respond_to_allowlist(&input.respond_to_allowlist)?; - if input.respond_to == crate::managed_agents::RespondTo::Allowlist + if input.respond_to == Some(crate::managed_agents::RespondTo::Allowlist) && respond_to_allowlist.is_empty() { return Err( @@ -742,13 +647,15 @@ pub async fn create_managed_agent( // (input.env_vars), and the live persona env is merged underneath at // read time (spawn / readiness / deploy) so persona credential edits // refresh on the next spawn like prompt/model/provider already do. - let persona_snapshot = requested_persona_id.as_deref().and_then(|pid| { + let linked_persona = requested_persona_id.as_deref().and_then(|pid| { load_personas(&app) .ok()? .into_iter() .find(|persona| persona.id == pid) - .map(|persona| crate::managed_agents::persona_events::persona_snapshot(&persona)) }); + let persona_snapshot = linked_persona + .as_ref() + .map(crate::managed_agents::persona_events::persona_snapshot); let snapshot_prompt = persona_snapshot .as_ref() .and_then(|s| s.system_prompt.clone()); @@ -756,6 +663,23 @@ pub async fn create_managed_agent( let snapshot_provider = persona_snapshot.as_ref().and_then(|s| s.provider.clone()); let snapshot_source_version = persona_snapshot.as_ref().map(|s| s.source_version.clone()); + // Mint-time behavioral quad: explicit input wins, then the linked + // definition's NIP-AP defaults, then client defaults. The ONLY parse + // point for definition behavioral strings — fails loudly on a bad + // mode/range instead of minting an agent the author didn't describe. + let minted = crate::managed_agents::resolve_mint_behavioral_defaults( + input.respond_to, + respond_to_allowlist.clone(), + input + .mcp_toolsets + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string), + input.parallelism, + linked_persona.as_ref(), + )?; + let record = crate::managed_agents::ManagedAgentRecord { pubkey: pubkey.clone(), name: name.clone(), @@ -782,10 +706,7 @@ pub async fn create_managed_agent( // 0 or None → harness uses its own default (320s idle, 3600s max), and the CLI also clamps 0 → minimum. idle_timeout_seconds: input.idle_timeout_seconds.filter(|s| *s > 0), max_turn_duration_seconds: input.max_turn_duration_seconds.filter(|s| *s > 0), - parallelism: input - .parallelism - .filter(|count| (1..=32).contains(count)) - .unwrap_or(DEFAULT_AGENT_PARALLELISM), + parallelism: minted.parallelism.unwrap_or(DEFAULT_AGENT_PARALLELISM), system_prompt: snapshot_prompt.or_else(|| { input .system_prompt @@ -811,12 +732,7 @@ pub async fn create_managed_agent( .map(str::to_string) }), persona_source_version: snapshot_source_version, - mcp_toolsets: input - .mcp_toolsets - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - .map(str::to_string), + mcp_toolsets: minted.mcp_toolsets.clone(), // Provider agents are managed externally — force false. start_on_app_launch: if input.backend != BackendKind::Local { false @@ -841,8 +757,8 @@ pub async fn create_managed_agent( last_exit_code: None, last_error: None, last_error_code: None, - respond_to: input.respond_to, - respond_to_allowlist: respond_to_allowlist.clone(), + respond_to: minted.respond_to, + respond_to_allowlist: minted.respond_to_allowlist.clone(), display_name: None, slug: None, runtime: None, @@ -851,6 +767,10 @@ pub async fn create_managed_agent( is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: relay_mesh.clone(), }; @@ -1256,6 +1176,12 @@ pub async fn delete_managed_agent( // 3. Desktop's existing presence polling sees "offline" — UI updates automatically // No backend Tauri command needed. Presence IS the status. +#[path = "agents_deploy.rs"] +mod deploy; +use deploy::build_deploy_payload; +#[cfg(test)] +use deploy::deploy_payload_json; + #[path = "agents_profile.rs"] mod profile; #[cfg(test)] diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs new file mode 100644 index 0000000000..47a01585cc --- /dev/null +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -0,0 +1,131 @@ +//! Provider deploy payload construction, split from `agents.rs` (file-size +//! guard). `build_deploy_payload` gathers live state; `deploy_payload_json` +//! is the pure serialization half so payload completeness stays testable. + +use tauri::AppHandle; + +use crate::{ + app_state::AppState, + managed_agents::{load_personas, ManagedAgentRecord}, + relay::relay_ws_url_with_override, +}; + +/// Build the standard agent JSON payload for provider deploy calls. +/// +/// Like local spawn, provider deploy re-reads live persona env vars and +/// structured model/provider so remote agents receive current credentials +/// and the same authoritative values that local spawn derives from +/// `runtime_metadata_env_vars`. The only field still pinned is +/// `agent_command`/`agent_args` — those were captured at create time. +/// The only read-time resolution is `relay_url`: a blank pin resolves to +/// the active workspace relay here, matching the create-path contract. +/// +/// Fails closed when the private key is unavailable (keyring outage leaves +/// it empty after hydration): without this guard a provider deploy would +/// serialize `"private_key_nsec": ""` and launch the agent with no +/// identity — the same hazard the local spawn path refuses via +/// `spawn_key_refusal`. +pub(super) fn build_deploy_payload( + app: &AppHandle, + state: &AppState, + record: &ManagedAgentRecord, +) -> Result { + // Fails closed when the private key is unavailable — same guard as local + // spawn. Without this, a keyring outage would serialize `"private_key_nsec": ""` + // and launch the agent with no identity. + if let Some(err) = crate::managed_agents::spawn_key_refusal(record) { + return Err(err); + } + + // Merge persona env_vars + agent env_vars for provider deploy — the same + // live-persona-under-overrides semantics as local spawn. Without this, + // provider-backed agents wouldn't receive credentials saved on the persona + // or the agent itself. + let persona_env = + crate::managed_agents::resolve_persona_env(app, record.persona_id.as_deref())?; + let merged_env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars); + + // Resolve the persona's structured provider/model so the remote provider + // receives the same authoritative values that local spawn derives from + // `runtime_metadata_env_vars`. Without this, remote deploy would rely on + // stale derived env copies in `env_vars` (or have no provider at all for + // imported personas whose derived keys were filtered at import time). + // + // Precedence: persona field wins when non-blank; otherwise falls back to the + // record's own field (same blank-normalization as the snapshot path). This + // matches `persona_snapshot_with_agent_config_fallback` exactly — a blank + // persona field must not wipe a record value that the user configured. + let (effective_model, effective_provider) = if let Some(pid) = record.persona_id.as_deref() { + let personas = load_personas(app).map_err(|e| { + format!( + "failed to load personas while building deploy payload for persona `{pid}`: {e}" + ) + })?; + let persona = personas + .into_iter() + .find(|p| p.id == pid) + .ok_or_else(|| format!("persona `{pid}` not found while building deploy payload"))?; + let fallback = crate::managed_agents::persona_events::persona_field_with_record_fallback; + let model = fallback(persona.model.as_deref(), record.model.as_deref()); // fallback: record.model + let provider = fallback(persona.provider.as_deref(), record.provider.as_deref()); // fallback: record.provider + (model, provider) + } else { + (record.model.clone(), record.provider.clone()) + }; + + Ok(deploy_payload_json( + record, + crate::relay::effective_agent_relay_url( + &record.relay_url, + &relay_ws_url_with_override(state), + ), + effective_model, + effective_provider, + merged_env, + )) +} + +/// Pure serialization half of [`build_deploy_payload`] — every field the +/// provider harness receives is deliberately listed here, so payload +/// completeness is testable without an `AppHandle`. +pub(super) fn deploy_payload_json( + record: &ManagedAgentRecord, + relay_url: String, + effective_model: Option, + effective_provider: Option, + merged_env: std::collections::BTreeMap, +) -> serde_json::Value { + serde_json::json!({ + "name": &record.name, + // Resolve the per-agent pin against the active workspace relay here: + // this payload crosses the host boundary to a remote provider harness + // that has no notion of the desktop's workspace, so the blank→workspace + // fallback (otherwise applied at read-time in `effective_agent_relay_url`) + // must be materialized into a concrete URL before serializing. + "relay_url": relay_url, + "private_key_nsec": &record.private_key_nsec, + "auth_tag": &record.auth_tag, + "agent_command": &record.agent_command, + "agent_args": &record.agent_args, + "system_prompt": &record.system_prompt, + "model": effective_model, + // Structured provider from the persona record. Providers that don't + // yet read this field will fall back to env_vars or their own default + // — no protocol break. + "provider": effective_provider, + "turn_timeout_seconds": record.turn_timeout_seconds, + "idle_timeout_seconds": record.idle_timeout_seconds, + "max_turn_duration_seconds": record.max_turn_duration_seconds, + "parallelism": record.parallelism, + // Inbound author gate. Providers that don't yet read these fall back + // to the harness default (`owner-only`) — no protocol break. + "respond_to": record.respond_to, + "respond_to_allowlist": &record.respond_to_allowlist, + // MCP toolset filter (BUZZ_TOOLSETS on the local spawn path). + // Providers that don't yet read this fall back to their default. + "mcp_toolsets": &record.mcp_toolsets, + // Merged persona + agent env vars. Providers that don't read this + // field will simply ignore it — no protocol break. + "env_vars": merged_env, + }) +} diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index fcb39095bc..1518517047 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -174,3 +174,54 @@ fn legacy_avatar_empty_when_nothing_resolves() { assert!(resolved.is_empty()); } + +// ── Provider deploy payload completeness ───────────────────────────────────── + +/// Regression (PR #1667 review, Thufir): the provider deploy payload must +/// carry every behavioral field the local spawn path applies — a field +/// missing here silently strips it from provider-backed agents. +#[test] +fn deploy_payload_carries_the_full_behavioral_quad() { + let allow = "a".repeat(64); + let record: ManagedAgentRecord = serde_json::from_str(&format!( + r#"{{ + "pubkey": "abcd1234", + "name": "test-agent", + "private_key_nsec": "nsec1fake", + "relay_url": "wss://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "parallelism": 4, + "mcp_toolsets": "developer,search", + "respond_to": "allowlist", + "respond_to_allowlist": ["{allow}"], + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }}"# + )) + .expect("sample record"); + + let payload = deploy_payload_json( + &record, + "wss://relay.example".to_string(), + Some("gpt-x".to_string()), + Some("openai".to_string()), + std::collections::BTreeMap::new(), + ); + + assert_eq!(payload["parallelism"], 4); + assert_eq!(payload["mcp_toolsets"], "developer,search"); + assert_eq!(payload["respond_to"], "allowlist"); + assert_eq!(payload["respond_to_allowlist"][0], "a".repeat(64)); + assert_eq!(payload["model"], "gpt-x"); + assert_eq!(payload["provider"], "openai"); + assert_eq!(payload["relay_url"], "wss://relay.example"); +} diff --git a/desktop/src-tauri/src/commands/personas/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound_tests.rs index d8e71aaad8..08d52a75b5 100644 --- a/desktop/src-tauri/src/commands/personas/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound_tests.rs @@ -23,6 +23,10 @@ fn local_in_app() -> PersonaRecord { source_team: Some("team-1".to_string()), source_team_persona_slug: None, env_vars: BTreeMap::from([("API_KEY".to_string(), "secret".to_string())]), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2025-01-01T00:00:00Z".to_string(), updated_at: "2025-01-01T00:00:00Z".to_string(), } @@ -45,6 +49,10 @@ fn inbound_for(d_tag: &str, display_name: &str) -> PersonaRecord { source_team: None, source_team_persona_slug: Some(d_tag.to_string()), env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2025-06-01T00:00:00Z".to_string(), updated_at: "2025-06-01T00:00:00Z".to_string(), } @@ -69,6 +77,36 @@ fn in_app_persona_matches_existing_uuid_and_patches() { assert_eq!(p.created_at, "2025-01-01T00:00:00Z"); } +#[test] +fn inbound_quad_edit_applies_to_existing_matched_record() { + // B5 quad activation: a remote quad edit must land on the MATCH branch, + // not just the insert branch — otherwise device B keeps its stale quad + // and its next reconcile republishes it over device A's edit, and the + // two devices never converge (permanent ping-pong). + let mut local = local_in_app(); + local.respond_to = Some("owner-only".to_string()); + local.parallelism = Some(2); + let mut personas = vec![local]; + + let mut inbound = inbound_for(UUID, "Remote"); + inbound.respond_to = Some("allowlist".to_string()); + inbound.respond_to_allowlist = vec!["a".repeat(64)]; + inbound.mcp_toolsets = Some("default,canvas".to_string()); + inbound.parallelism = Some(8); + apply_inbound_persona(&mut personas, inbound); + + assert_eq!(personas.len(), 1, "no duplicate row"); + let p = &personas[0]; + assert_eq!(p.respond_to, Some("allowlist".to_string())); + assert_eq!(p.respond_to_allowlist, vec!["a".repeat(64)]); + assert_eq!(p.mcp_toolsets, Some("default,canvas".to_string())); + assert_eq!(p.parallelism, Some(8)); + // A quad-absent inbound also applies (clears), same as prompt/model. + apply_inbound_persona(&mut personas, inbound_for(UUID, "Remote")); + assert_eq!(personas[0].respond_to, None); + assert_eq!(personas[0].parallelism, None); +} + #[test] fn re_received_in_app_persona_is_idempotent_no_duplicate() { let mut personas = vec![local_in_app()]; @@ -171,6 +209,10 @@ fn local_agent() -> ManagedAgentRecord { is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index 76230b5d00..c3fb76d0dd 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -5,10 +5,10 @@ use super::export_util::save_json_with_dialog; use crate::{ app_state::AppState, managed_agents::{ - agent_events::ManagedAgentEventContent, effective_agent_command, encode_persona_json, - load_managed_agents, load_personas, load_teams, managed_agent_avatar_url, - parse_json_persona, parse_md_persona, parse_png_persona, parse_zip_personas, - persona_events::persona_d_tag, save_managed_agents, save_personas, + agent_events::ManagedAgentEventContent, apply_persona_behavior, effective_agent_command, + encode_persona_json, load_managed_agents, load_personas, load_teams, + managed_agent_avatar_url, parse_json_persona, parse_md_persona, parse_png_persona, + parse_zip_personas, persona_events::persona_d_tag, save_managed_agents, save_personas, team_events::TeamEventContent, team_persona_key, try_regenerate_nest, validate_persona_activation_change, validate_persona_deletion, CreatePersonaRequest, ManagedAgentRecord, ParsePersonaFilesResult, PersonaRecord, TeamRecord, @@ -32,123 +32,9 @@ fn trim_optional(value: Option) -> Option { }) } -/// Retain a freshly authored persona event in the local store, flagged for -/// relay sync. Called inside a command's `managed_agents_store_lock`-held body -/// after `save_personas`; the background flush loop publishes it out-of-band. -/// -/// The event is signed with the owner keys at call time, so its `created_at` -/// is `now` — newer than any prior retained row, clearing the upsert's -/// newer-or-equal guard. `pending_sync = 1` enqueues it for the flush loop, -/// which is the sole publisher. Best-effort: a failure here is logged and -/// swallowed so a retention hiccup never blocks the disk-authoritative write. -/// -/// Unlike `retain_managed_agent_pending`, this has no projection-equality -/// short-circuit: personas have no start/stop runtime churn, so a republish -/// only happens on a genuine create/update/delete user edit (`set_persona_active` -/// does not retain, so the local-only `is_active` toggle never republishes, and -/// a byte-identical user-save republish is harmlessly NIP-33-replaced). The -/// guard is intentionally omitted. -fn retain_persona_pending(app: &AppHandle, state: &AppState, persona: &PersonaRecord) { - use crate::managed_agents::{ - managed_agents_base_dir, - persona_events::{build_persona_event, monotonic_created_at, persona_d_tag}, - retention::{get_retained_event, open_retention_db, retain_event, RetainedEvent}, - }; - use buzz_core_pkg::kind::KIND_PERSONA; - use nostr::JsonUtil; - - let result = (|| -> Result<(), String> { - let d_tag = persona_d_tag(persona); - let conn = open_retention_db(&managed_agents_base_dir(app)?.join("retention.db"))?; - let (pubkey, event) = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; - // Monotonic created_at: read the retained head for this coordinate - // and bump past it (NIP-AP step 3) so a same-second edit supersedes. - let prior = - get_retained_event(&conn, KIND_PERSONA, &keys.public_key().to_hex(), &d_tag)? - .map(|row| row.created_at); - let event = build_persona_event(persona)? - .custom_created_at(monotonic_created_at(prior)) - .sign_with_keys(&keys) - .map_err(|e| format!("failed to sign persona event: {e}"))?; - (keys.public_key().to_hex(), event) - }; - retain_event( - &conn, - &RetainedEvent { - kind: KIND_PERSONA, - pubkey, - d_tag, - content: event.content.to_string(), - created_at: event.created_at.as_secs() as i64, - raw_event: event.as_json(), - pending_sync: true, - }, - ) - })(); - if let Err(e) = result { - eprintln!("buzz-desktop: persona-retain: {e}"); - } -} - -/// Purge a deleted persona's pending row and enqueue a NIP-09 tombstone, both -/// inside the `managed_agents_store_lock`-held delete body. -/// -/// PURGE IN: `delete_retained_event` removes the persona's `(30175, pubkey, -/// d_tag)` row. Running it under the same lock that serializes `retain_event` -/// closes the same-second resurrect race — a concurrent edit can't re-insert a -/// pending persona row after the tombstone is queued. -/// -/// PUBLISH OUT: the kind:5 tombstone is retained at its own coordinate `(5, -/// pubkey, d_tag)` (distinct from the purged persona row) with `pending_sync = -/// 1`; the flush loop publishes it. Best-effort: a failure is logged and -/// swallowed so a retention hiccup never blocks the disk-authoritative delete. -pub(super) fn tombstone_persona_pending(app: &AppHandle, state: &AppState, d_tag: &str) { - use crate::managed_agents::{ - managed_agents_base_dir, - persona_events::build_persona_delete, - retention::{ - delete_retained_event, open_retention_db, retain_event, tombstone_retention_d_tag, - RetainedEvent, - }, - }; - use buzz_core_pkg::kind::KIND_PERSONA; - use nostr::JsonUtil; - - const KIND_DELETE: u32 = 5; - - let result = (|| -> Result<(), String> { - let (pubkey, event) = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; - let pubkey = keys.public_key().to_hex(); - let event = build_persona_delete(d_tag, &pubkey)? - .sign_with_keys(&keys) - .map_err(|e| format!("failed to sign persona tombstone: {e}"))?; - (pubkey, event) - }; - let conn = open_retention_db(&managed_agents_base_dir(app)?.join("retention.db"))?; - // Purge the persona row first so an unpublished edit can never resurrect - // it after the tombstone publishes. - delete_retained_event(&conn, KIND_PERSONA, &pubkey, d_tag)?; - retain_event( - &conn, - &RetainedEvent { - kind: KIND_DELETE, - pubkey, - // Key by the target coordinate so cross-kind d-tag tombstones - // occupy distinct rows (F2c). - d_tag: tombstone_retention_d_tag(KIND_PERSONA, d_tag), - content: event.content.to_string(), - created_at: event.created_at.as_secs() as i64, - raw_event: event.as_json(), - pending_sync: true, - }, - ) - })(); - if let Err(e) = result { - eprintln!("buzz-desktop: persona-tombstone: {e}"); - } -} +mod pending; +use pending::retain_persona_pending; +pub(super) use pending::tombstone_persona_pending; #[tauri::command] pub async fn list_personas(app: AppHandle) -> Result, String> { @@ -193,7 +79,7 @@ pub async fn create_persona( .filter(|s| !s.is_empty()) .collect(); crate::managed_agents::validate_user_env_keys(&input.env_vars)?; - let persona = PersonaRecord { + let mut persona = PersonaRecord { id: Uuid::new_v4().to_string(), display_name, avatar_url, @@ -207,9 +93,14 @@ pub async fn create_persona( source_team: None, source_team_persona_slug: None, env_vars: input.env_vars, + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: now.clone(), updated_at: now, }; + apply_persona_behavior(&mut persona, input.behavior)?; personas.push(persona.clone()); save_personas(&app, &personas)?; retain_persona_pending(&app, &state, &persona); @@ -279,6 +170,7 @@ pub async fn update_persona( crate::managed_agents::validate_user_env_keys(&env_vars)?; persona.env_vars = env_vars; } + apply_persona_behavior(persona, input.behavior)?; persona.updated_at = now_iso(); save_personas(&app, &personas)?; @@ -721,6 +613,10 @@ fn apply_inbound_persona(personas: &mut Vec, inbound: PersonaReco local.model = inbound.model; local.provider = inbound.provider; local.name_pool = inbound.name_pool; + local.respond_to = inbound.respond_to; + local.respond_to_allowlist = inbound.respond_to_allowlist; + local.mcp_toolsets = inbound.mcp_toolsets; + local.parallelism = inbound.parallelism; local.updated_at = inbound.updated_at; } None => personas.push(inbound), diff --git a/desktop/src-tauri/src/commands/personas/pending.rs b/desktop/src-tauri/src/commands/personas/pending.rs new file mode 100644 index 0000000000..a01d380871 --- /dev/null +++ b/desktop/src-tauri/src/commands/personas/pending.rs @@ -0,0 +1,130 @@ +//! Retention-store enqueue helpers for owner-authored persona writes: retain +//! a pending 30175 on create/update, purge + tombstone on delete. The flush +//! loop (`flush_pending_events`) is the sole publisher. + +use tauri::AppHandle; + +use crate::app_state::AppState; +use crate::managed_agents::PersonaRecord; + +/// Retain a freshly authored persona event in the local store, flagged for +/// relay sync. Called inside a command's `managed_agents_store_lock`-held body +/// after `save_personas`; the background flush loop publishes it out-of-band. +/// +/// The event is signed with the owner keys at call time, so its `created_at` +/// is `now` — newer than any prior retained row, clearing the upsert's +/// newer-or-equal guard. `pending_sync = 1` enqueues it for the flush loop, +/// which is the sole publisher. Best-effort: a failure here is logged and +/// swallowed so a retention hiccup never blocks the disk-authoritative write. +/// +/// Unlike `retain_managed_agent_pending`, this has no projection-equality +/// short-circuit: personas have no start/stop runtime churn, so a republish +/// only happens on a genuine create/update/delete user edit (`set_persona_active` +/// does not retain, so the local-only `is_active` toggle never republishes, and +/// a byte-identical user-save republish is harmlessly NIP-33-replaced). The +/// guard is intentionally omitted. +pub(super) fn retain_persona_pending(app: &AppHandle, state: &AppState, persona: &PersonaRecord) { + use crate::managed_agents::{ + managed_agents_base_dir, + persona_events::{build_persona_event, monotonic_created_at, persona_d_tag}, + retention::{get_retained_event, open_retention_db, retain_event, RetainedEvent}, + }; + use buzz_core_pkg::kind::KIND_PERSONA; + use nostr::JsonUtil; + + let result = (|| -> Result<(), String> { + let d_tag = persona_d_tag(persona); + let conn = open_retention_db(&managed_agents_base_dir(app)?.join("retention.db"))?; + let (pubkey, event) = { + let keys = state.keys.lock().map_err(|e| e.to_string())?; + // Monotonic created_at: read the retained head for this coordinate + // and bump past it (NIP-AP step 3) so a same-second edit supersedes. + let prior = + get_retained_event(&conn, KIND_PERSONA, &keys.public_key().to_hex(), &d_tag)? + .map(|row| row.created_at); + let event = build_persona_event(persona)? + .custom_created_at(monotonic_created_at(prior)) + .sign_with_keys(&keys) + .map_err(|e| format!("failed to sign persona event: {e}"))?; + (keys.public_key().to_hex(), event) + }; + retain_event( + &conn, + &RetainedEvent { + kind: KIND_PERSONA, + pubkey, + d_tag, + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: true, + }, + ) + })(); + if let Err(e) = result { + eprintln!("buzz-desktop: persona-retain: {e}"); + } +} + +/// Purge a deleted persona's pending row and enqueue a NIP-09 tombstone, both +/// inside the `managed_agents_store_lock`-held delete body. +/// +/// PURGE IN: `delete_retained_event` removes the persona's `(30175, pubkey, +/// d_tag)` row. Running it under the same lock that serializes `retain_event` +/// closes the same-second resurrect race — a concurrent edit can't re-insert a +/// pending persona row after the tombstone is queued. +/// +/// PUBLISH OUT: the kind:5 tombstone is retained at its own coordinate `(5, +/// pubkey, d_tag)` (distinct from the purged persona row) with `pending_sync = +/// 1`; the flush loop publishes it. Best-effort: a failure is logged and +/// swallowed so a retention hiccup never blocks the disk-authoritative delete. +pub(in crate::commands) fn tombstone_persona_pending( + app: &AppHandle, + state: &AppState, + d_tag: &str, +) { + use crate::managed_agents::{ + managed_agents_base_dir, + persona_events::build_persona_delete, + retention::{ + delete_retained_event, open_retention_db, retain_event, tombstone_retention_d_tag, + RetainedEvent, + }, + }; + use buzz_core_pkg::kind::KIND_PERSONA; + use nostr::JsonUtil; + + const KIND_DELETE: u32 = 5; + + let result = (|| -> Result<(), String> { + let (pubkey, event) = { + let keys = state.keys.lock().map_err(|e| e.to_string())?; + let pubkey = keys.public_key().to_hex(); + let event = build_persona_delete(d_tag, &pubkey)? + .sign_with_keys(&keys) + .map_err(|e| format!("failed to sign persona tombstone: {e}"))?; + (pubkey, event) + }; + let conn = open_retention_db(&managed_agents_base_dir(app)?.join("retention.db"))?; + // Purge the persona row first so an unpublished edit can never resurrect + // it after the tombstone publishes. + delete_retained_event(&conn, KIND_PERSONA, &pubkey, d_tag)?; + retain_event( + &conn, + &RetainedEvent { + kind: KIND_DELETE, + pubkey, + // Key by the target coordinate so cross-kind d-tag tombstones + // occupy distinct rows (F2c). + d_tag: tombstone_retention_d_tag(KIND_PERSONA, d_tag), + content: event.content.to_string(), + created_at: event.created_at.as_secs() as i64, + raw_event: event.as_json(), + pending_sync: true, + }, + ) + })(); + if let Err(e) = result { + eprintln!("buzz-desktop: persona-tombstone: {e}"); + } +} diff --git a/desktop/src-tauri/src/commands/personas/writeback.rs b/desktop/src-tauri/src/commands/personas/writeback.rs index 03db967630..e5d4fa9618 100644 --- a/desktop/src-tauri/src/commands/personas/writeback.rs +++ b/desktop/src-tauri/src/commands/personas/writeback.rs @@ -279,6 +279,10 @@ mod writeback_tests { source_team: Some("team-1".to_string()), source_team_persona_slug: Some("paul".to_string()), env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2025-01-01T00:00:00Z".to_string(), updated_at: "2025-01-01T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index 01d186cc09..e727b2e06f 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -213,6 +213,10 @@ mod tests { is_active: false, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, } } 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 dc5a5e287d..1488ecc973 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 @@ -85,6 +85,10 @@ fn test_record() -> ManagedAgentRecord { is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, agent_command_override: None, persona_source_version: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 8d649dca96..260bc9e3b8 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -201,6 +201,10 @@ fn persona_with_runtime(id: &str, runtime: Option<&str>) -> crate::managed_agent source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2026-06-09T00:00:00Z".to_string(), updated_at: "2026-06-09T00:00:00Z".to_string(), } @@ -271,6 +275,10 @@ fn record_with( is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index a81c5b09db..6f3d37e0a2 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -425,6 +425,10 @@ fn make_persona(id: &str, display_name: &str) -> PersonaRecord { source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: String::new(), updated_at: String::new(), } @@ -479,6 +483,10 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index d9722e2293..faa9507989 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -193,6 +193,10 @@ pub fn persona_from_event(event: &nostr::Event) -> Result source_team: None, source_team_persona_slug: Some(d_tag), env_vars: BTreeMap::new(), + respond_to: content.respond_to, + respond_to_allowlist: content.respond_to_allowlist, + mcp_toolsets: content.mcp_toolsets, + parallelism: content.parallelism, created_at: created_at.clone(), updated_at: created_at, }) @@ -222,7 +226,8 @@ pub async fn flush_pending_events( state: &AppState, ) -> Result { use crate::managed_agents::retention::{ - get_pending_sync, get_retained_event, mark_synced, open_retention_db, + deferred_behind_failed_tombstone, get_pending_sync, get_retained_event, mark_synced, + open_retention_db, }; use nostr::JsonUtil; @@ -232,7 +237,12 @@ pub async fn flush_pending_events( }; // connection dropped before any .await let mut flushed = 0u32; + let mut failed_tombstones: std::collections::HashSet<(String, String)> = + std::collections::HashSet::new(); for row in pending { + if deferred_behind_failed_tombstone(row.kind, &row.pubkey, &row.d_tag, &failed_tombstones) { + continue; // its tombstone failed this sweep; next sweep re-orders them + } // Re-read immediately before publishing; the row may have been edited // or deleted since the pending snapshot above. let current = { @@ -253,6 +263,9 @@ pub async fn flush_pending_events( .await .is_err() { + if current.kind == 5 { + failed_tombstones.insert((current.pubkey.clone(), current.d_tag.clone())); + } continue; // relay unreachable — stays pending for the next sweep } @@ -300,14 +313,15 @@ pub fn persona_event_content(record: &PersonaRecord) -> PersonaEventContent { model: record.model.clone(), provider: record.provider.clone(), name_pool: record.name_pool.clone(), - // NIP-AP behavioral defaults: RESERVED this release — parsed at the - // wire layer but not yet carried on PersonaRecord or applied at - // instance creation (that lands with the create-path unification). - // Guarded by `behavioral_defaults_are_staged_not_applied`. - respond_to: None, - respond_to_allowlist: Vec::new(), - mcp_toolsets: None, - parallelism: None, + // NIP-AP behavioral defaults: live since the create-path unification + // (B5) — carried on PersonaRecord in wire shape and copied verbatim. + // Quad-absent records serialize identically to the reserved era, so + // persona_content_hash is stable across the activation (guarded by + // `quad_absent_definition_hash_stable_across_activation`). + respond_to: record.respond_to.clone(), + respond_to_allowlist: record.respond_to_allowlist.clone(), + mcp_toolsets: record.mcp_toolsets.clone(), + parallelism: record.parallelism, } } @@ -396,584 +410,5 @@ pub fn persona_snapshot_with_agent_config_fallback( ..base } } - #[cfg(test)] -mod tests { - use super::*; - - fn sample_persona() -> PersonaRecord { - PersonaRecord { - id: "test-persona".to_string(), - display_name: "Test Persona".to_string(), - avatar_url: Some("https://example.com/avatar.png".to_string()), - system_prompt: "You are a test assistant.".to_string(), - runtime: Some("goose".to_string()), - model: Some("claude-opus-4".to_string()), - provider: Some("anthropic".to_string()), - name_pool: vec!["Alpha".to_string(), "Beta".to_string()], - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: Some("test-slug".to_string()), - env_vars: BTreeMap::from([("KEY".to_string(), "value".to_string())]), - created_at: "2025-01-01T00:00:00Z".to_string(), - updated_at: "2025-01-01T00:00:00Z".to_string(), - } - } - - #[test] - fn monotonic_created_at_bumps_past_head() { - // No head: uses now (floor 0). - let now = nostr::Timestamp::now().as_secs() as i64; - let none = monotonic_created_at(None).as_secs() as i64; - assert!(none >= now, "no-head write must be >= now"); - - // Head in the FUTURE (same-second or clock-skewed): must bump to head+1, - // never reuse now (which would be <= head and lose the NIP-33 tiebreak). - let future_head = now + 1000; - let bumped = monotonic_created_at(Some(future_head)).as_secs() as i64; - assert_eq!( - bumped, - future_head + 1, - "must supersede a future head by +1" - ); - - // Head in the PAST: now already exceeds it, so now wins. - let past = monotonic_created_at(Some(now - 1000)).as_secs() as i64; - assert!(past >= now, "past head must not drag created_at backward"); - } - - #[test] - fn d_tag_uses_slug_when_available() { - let record = sample_persona(); - assert_eq!(persona_d_tag(&record), "test-slug"); - } - - #[test] - fn d_tag_falls_back_to_id() { - let mut record = sample_persona(); - record.source_team_persona_slug = None; - assert_eq!(persona_d_tag(&record), "test-persona"); - } - - /// Mirror of the relay slug grammar (`ingest.rs:923` `^[a-z0-9][a-z0-9_-]{0,63}$`) - /// so the normalization tests assert what the relay actually enforces. - fn passes_relay_slug_grammar(d: &str) -> bool { - let bytes = d.as_bytes(); - !d.is_empty() - && d.len() <= 64 - && (bytes[0].is_ascii_lowercase() || bytes[0].is_ascii_digit()) - && bytes[1..] - .iter() - .all(|&b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'_' || b == b'-') - } - - #[test] - fn d_tag_normalizes_pack_slug_to_relay_grammar() { - // The cited failing cases: mixed-case and leading-underscore pack slugs - // that the relay rejects un-normalized → pending forever. - for (raw, expected) in [ - ("CodeReviewer", "codereviewer"), - ("_ops", "a_ops"), - ("Code-Reviewer", "code-reviewer"), - ("UPPER_snake", "upper_snake"), - ("-leading-dash", "a-leading-dash"), - ] { - let mut record = sample_persona(); - record.source_team_persona_slug = Some(raw.to_string()); - let d = persona_d_tag(&record); - assert_eq!(d, expected, "normalization of {raw:?}"); - assert!( - passes_relay_slug_grammar(&d), - "normalized {raw:?} -> {d:?} still fails the relay grammar" - ); - } - } - - #[test] - fn d_tag_already_valid_slug_is_unchanged() { - // In-app personas use a lowercase-hex UUID id — already valid, must pass - // through untouched (no spurious coordinate change on existing data). - let mut record = sample_persona(); - record.source_team_persona_slug = None; - record.id = "11111111-2222-3333-4444-555555555555".to_string(); - let d = persona_d_tag(&record); - assert_eq!(d, "11111111-2222-3333-4444-555555555555"); - assert!(passes_relay_slug_grammar(&d)); - } - - #[test] - fn build_persona_event_produces_correct_kind() { - let record = sample_persona(); - let builder = build_persona_event(&record).unwrap(); - let keys = nostr::Keys::generate(); - let event = builder.sign_with_keys(&keys).unwrap(); - assert_eq!(event.kind.as_u16() as u32, KIND_PERSONA); - } - - #[test] - fn round_trip_serialization() { - let record = sample_persona(); - let builder = build_persona_event(&record).unwrap(); - let keys = nostr::Keys::generate(); - let event = builder.sign_with_keys(&keys).unwrap(); - - let restored = persona_from_event(&event).unwrap(); - assert_eq!(restored.id, "test-slug"); - assert_eq!(restored.display_name, "Test Persona"); - assert_eq!( - restored.avatar_url, - Some("https://example.com/avatar.png".to_string()) - ); - assert_eq!(restored.system_prompt, "You are a test assistant."); - assert_eq!(restored.runtime, Some("goose".to_string())); - assert_eq!(restored.model, Some("claude-opus-4".to_string())); - assert_eq!(restored.provider, Some("anthropic".to_string())); - assert_eq!(restored.name_pool, vec!["Alpha", "Beta"]); - // env_vars are not included in public persona events (secrets travel - // via NIP-44-encrypted engrams only). - assert!(restored.env_vars.is_empty()); - assert_eq!( - restored.source_team_persona_slug, - Some("test-slug".to_string()) - ); - assert!(!restored.is_builtin); - assert!(restored.is_active); - } - - /// NIP-AP reference vector (Event 1, `docs/nips/NIP-AP.md:195-207`): the - /// serialized content bytes MUST match the spec exactly, byte-for-byte. - /// serde emits fields in declaration order, so this pins the content - /// encoding — and therefore the NIP-01 event id — for cross-implementation - /// interop. The field order is `display_name, system_prompt, avatar_url, - /// runtime, model, provider, name_pool`. - #[test] - fn content_matches_nip_ap_vector() { - // Exact body from NIP-AP.md Event 1 (no trailing whitespace, no BOM). - const VECTOR: &str = r#"{"display_name":"Test Agent","system_prompt":"You are a test assistant.","avatar_url":"https://example.com/avatar.png","runtime":"goose","model":"claude-opus-4","provider":"anthropic","name_pool":["Alpha","Beta"]}"#; - - let content = PersonaEventContent { - display_name: "Test Agent".to_string(), - system_prompt: Some("You are a test assistant.".to_string()), - avatar_url: Some("https://example.com/avatar.png".to_string()), - runtime: Some("goose".to_string()), - model: Some("claude-opus-4".to_string()), - provider: Some("anthropic".to_string()), - name_pool: vec!["Alpha".to_string(), "Beta".to_string()], - respond_to: None, - respond_to_allowlist: Vec::new(), - mcp_toolsets: None, - parallelism: None, - }; - assert_eq!( - serde_json::to_string(&content).unwrap(), - VECTOR, - "serialized content drifted from the NIP-AP Event 1 vector" - ); - - // Hash invariance across the unified-model widening: REAL pre-revision - // content bytes (fixture string, not a round-trip through the new - // struct) must parse and re-serialize byte-identically, so - // persona_content_hash — the drift-badge basis — is unchanged on - // upgrade. A bare Option serializing "system_prompt":null would flip - // every persona's hash fleet-wide. - let parsed: PersonaEventContent = serde_json::from_str(VECTOR).unwrap(); - assert_eq!( - serde_json::to_string(&parsed).unwrap(), - VECTOR, - "pre-revision content bytes must survive a parse/serialize round-trip unchanged" - ); - assert_eq!( - persona_content_hash(&parsed), - { - use sha2::{Digest, Sha256}; - hex::encode(Sha256::digest(VECTOR.as_bytes())) - }, - "persona_content_hash of pre-revision bytes must equal the direct digest" - ); - - // Hash stability, adversarial shapes: the empty prompt and the - // minimal old-writer body (display_name + system_prompt only) are the - // two easiest regressions if the projection or skip attributes ever - // change. - const EMPTY_PROMPT: &str = r#"{"display_name":"X","system_prompt":""}"#; - let parsed: PersonaEventContent = serde_json::from_str(EMPTY_PROMPT).unwrap(); - assert_eq!(serde_json::to_string(&parsed).unwrap(), EMPTY_PROMPT); - const MINIMAL: &str = r#"{"display_name":"Minimal","system_prompt":"Hello."}"#; - let parsed: PersonaEventContent = serde_json::from_str(MINIMAL).unwrap(); - assert_eq!(serde_json::to_string(&parsed).unwrap(), MINIMAL); - - // An event built from this content carries the byte-exact vector as its - // signed content, so a second implementer following the spec computes - // the same NIP-01 id. - let record = PersonaRecord { - id: "test-agent".to_string(), - display_name: "Test Agent".to_string(), - avatar_url: Some("https://example.com/avatar.png".to_string()), - system_prompt: "You are a test assistant.".to_string(), - runtime: Some("goose".to_string()), - model: Some("claude-opus-4".to_string()), - provider: Some("anthropic".to_string()), - name_pool: vec!["Alpha".to_string(), "Beta".to_string()], - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - env_vars: BTreeMap::new(), - created_at: "2025-01-01T00:00:00Z".to_string(), - updated_at: "2025-01-01T00:00:00Z".to_string(), - }; - let event = build_persona_event(&record) - .unwrap() - .sign_with_keys(&nostr::Keys::generate()) - .unwrap(); - assert_eq!(event.content, VECTOR); - } - - #[test] - fn round_trip_minimal_persona() { - let record = PersonaRecord { - id: "minimal".to_string(), - display_name: "Minimal".to_string(), - avatar_url: None, - system_prompt: "Hello".to_string(), - runtime: None, - model: None, - provider: None, - name_pool: vec![], - is_builtin: true, - is_active: false, - source_team: Some("team-1".to_string()), - source_team_persona_slug: None, - env_vars: BTreeMap::new(), - created_at: "2025-01-01T00:00:00Z".to_string(), - updated_at: "2025-01-01T00:00:00Z".to_string(), - }; - - let builder = build_persona_event(&record).unwrap(); - let keys = nostr::Keys::generate(); - let event = builder.sign_with_keys(&keys).unwrap(); - - let restored = persona_from_event(&event).unwrap(); - assert_eq!(restored.id, "minimal"); - assert_eq!(restored.display_name, "Minimal"); - assert_eq!(restored.avatar_url, None); - assert_eq!(restored.runtime, None); - assert_eq!(restored.model, None); - assert_eq!(restored.provider, None); - assert!(restored.name_pool.is_empty()); - assert!(restored.env_vars.is_empty()); - // Deserialized persona is always non-builtin and active - assert!(!restored.is_builtin); - assert!(restored.is_active); - } - - #[test] - fn build_persona_delete_has_single_a_tag_no_e_tag() { - const OWNER: &str = "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798"; - let builder = build_persona_delete("test-slug", OWNER).unwrap(); - let keys = nostr::Keys::generate(); - let event = builder.sign_with_keys(&keys).unwrap(); - - assert_eq!(event.kind, Kind::Custom(5)); - - let a_tags: Vec<&[String]> = event - .tags - .iter() - .map(|t| t.as_slice()) - .filter(|v| v.first().map(String::as_str) == Some("a")) - .collect(); - assert_eq!(a_tags.len(), 1); - assert_eq!(a_tags[0][1], format!("{KIND_PERSONA}:{OWNER}:test-slug")); - - // An e-tag would route to the event-id deletion path and leave the - // replaceable coordinate live — the tombstone must carry none. - assert!(event - .tags - .iter() - .all(|t| t.as_slice().first().map(String::as_str) != Some("e"))); - } - - /// NIP-AP behavioral defaults are RESERVED this release: the wire type - /// parses them (foreign data survives deserialization) but the local - /// projection does not emit them and PersonaRecord does not carry them. - /// This test locks the staged behavior so activating the fields later is - /// a deliberate act — and documents that a foreign definition's - /// behavioral values do NOT survive a local edit-and-republish cycle yet. - #[test] - fn behavioral_defaults_are_staged_not_applied() { - const FOREIGN: &str = r#"{"display_name":"F","system_prompt":"p","respond_to":"anyone","respond_to_allowlist":["deadbeef"],"mcp_toolsets":"default","parallelism":4}"#; - let parsed: PersonaEventContent = serde_json::from_str(FOREIGN).unwrap(); - // Wire layer preserves the fields... - assert_eq!(parsed.respond_to.as_deref(), Some("anyone")); - assert_eq!(parsed.parallelism, Some(4)); - // ...but the record round-trip drops them (staged, not applied). - let record = persona_from_event_content_for_test(parsed); - let reprojected = persona_event_content(&record); - assert_eq!(reprojected.respond_to, None); - assert_eq!(reprojected.respond_to_allowlist, Vec::::new()); - assert_eq!(reprojected.mcp_toolsets, None); - assert_eq!(reprojected.parallelism, None); - } - - /// Test-only bridge: build a PersonaRecord from parsed content the same - /// way `persona_from_event` maps fields, without needing a signed event. - fn persona_from_event_content_for_test(content: PersonaEventContent) -> PersonaRecord { - PersonaRecord { - id: "staged".to_string(), - display_name: content.display_name, - avatar_url: content.avatar_url, - system_prompt: content.system_prompt.unwrap_or_default(), - runtime: content.runtime, - model: content.model, - provider: content.provider, - name_pool: content.name_pool, - is_builtin: false, - is_active: true, - source_team: None, - source_team_persona_slug: None, - env_vars: BTreeMap::new(), - created_at: "2026-01-01T00:00:00Z".to_string(), - updated_at: "2026-01-01T00:00:00Z".to_string(), - } - } - - #[test] - fn persona_content_hash_is_deterministic() { - let content = PersonaEventContent { - display_name: "Test".to_string(), - avatar_url: None, - system_prompt: Some("Hello".to_string()), - runtime: None, - model: None, - provider: None, - name_pool: vec![], - respond_to: None, - respond_to_allowlist: Vec::new(), - mcp_toolsets: None, - parallelism: None, - }; - let hash1 = persona_content_hash(&content); - let hash2 = persona_content_hash(&content); - assert_eq!(hash1, hash2); - assert_eq!(hash1.len(), 64); // SHA-256 hex - } - - #[test] - fn persona_content_hash_changes_on_edit() { - let content1 = PersonaEventContent { - display_name: "Test".to_string(), - avatar_url: None, - system_prompt: Some("Hello".to_string()), - runtime: None, - model: None, - provider: None, - name_pool: vec![], - respond_to: None, - respond_to_allowlist: Vec::new(), - mcp_toolsets: None, - parallelism: None, - }; - let mut content2 = content1.clone(); - content2.system_prompt = Some("Goodbye".to_string()); - assert_ne!( - persona_content_hash(&content1), - persona_content_hash(&content2) - ); - } - - // ── persona_field_with_record_fallback ──────────────────────────────────── - - #[test] - fn field_fallback_persona_present_wins() { - assert_eq!( - persona_field_with_record_fallback(Some("persona-model"), Some("record-model")), - Some("persona-model".to_owned()), - ); - } - - #[test] - fn field_fallback_persona_blank_uses_record() { - assert_eq!( - persona_field_with_record_fallback(None, Some("record-model")), - Some("record-model".to_owned()), - ); - assert_eq!( - persona_field_with_record_fallback(Some(" "), Some("record-model")), - Some("record-model".to_owned()), - ); - } - - #[test] - fn field_fallback_both_blank_is_none() { - assert_eq!(persona_field_with_record_fallback(None, None), None); - assert_eq!(persona_field_with_record_fallback(Some(""), Some("")), None); - } - - #[test] - fn field_fallback_record_blank_is_none() { - assert_eq!( - persona_field_with_record_fallback(None, Some(" ")), - None, - "whitespace-only record value must also be treated as blank" - ); - } - - // ── persona_snapshot_with_agent_config_fallback ──────────────────────────── - - /// Helper: a persona with no model/provider configured. - fn blank_model_persona() -> PersonaRecord { - PersonaRecord { - model: None, - provider: None, - ..sample_persona() - } - } - - /// (a) Persona leaves model/provider blank, agent record has values → - /// record values preserved AND source_version still updated to current hash. - #[test] - fn fallback_preserves_record_values_when_persona_blank() { - let persona = blank_model_persona(); - let expected_version = persona_content_hash(&persona_event_content(&persona)); - - let snapshot = - persona_snapshot_with_agent_config_fallback(&persona, Some("gpt-4o"), Some("openai")); - - assert_eq!( - snapshot.model.as_deref(), - Some("gpt-4o"), - "blank persona model must fall back to agent record value" - ); - assert_eq!( - snapshot.provider.as_deref(), - Some("openai"), - "blank persona provider must fall back to agent record value" - ); - assert_eq!( - snapshot.source_version, expected_version, - "source_version must still reflect current persona hash" - ); - } - - /// (b) Persona has model/provider set → persona wins over agent record. - #[test] - fn fallback_persona_wins_when_set() { - let persona = sample_persona(); // has model=Some("claude-opus-4"), provider=Some("anthropic") - - let snapshot = persona_snapshot_with_agent_config_fallback( - &persona, - Some("gpt-4o"), // agent had a different model - Some("openai"), // agent had a different provider - ); - - assert_eq!( - snapshot.model.as_deref(), - Some("claude-opus-4"), - "persona model must win when persona has a value" - ); - assert_eq!( - snapshot.provider.as_deref(), - Some("anthropic"), - "persona provider must win when persona has a value" - ); - } - - /// (c) Both blank → snapshot keeps None; a genuinely unconfigured agent - /// stays unconfigured (no fabricated values). - #[test] - fn fallback_both_blank_stays_none() { - let persona = blank_model_persona(); - - let snapshot = persona_snapshot_with_agent_config_fallback( - &persona, None, // agent also has no model - None, // agent also has no provider - ); - - assert!( - snapshot.model.is_none(), - "neither persona nor agent has model — snapshot must be None" - ); - assert!( - snapshot.provider.is_none(), - "neither persona nor agent has provider — snapshot must be None" - ); - } - - /// Whitespace-only values on the persona are treated as blank; agent - /// fallback applies. - #[test] - fn fallback_treats_whitespace_only_persona_value_as_blank() { - let mut persona = sample_persona(); - persona.model = Some(" ".to_string()); - persona.provider = Some("\t".to_string()); - - let snapshot = persona_snapshot_with_agent_config_fallback( - &persona, - Some("claude-opus-4"), - Some("anthropic"), - ); - - assert_eq!( - snapshot.model.as_deref(), - Some("claude-opus-4"), - "whitespace-only persona model must be treated as blank" - ); - assert_eq!( - snapshot.provider.as_deref(), - Some("anthropic"), - "whitespace-only persona provider must be treated as blank" - ); - } - - /// Cross-field independence: persona sets model but not provider → model - /// comes from persona, provider falls back to the record. This is the - /// practically common case (model-only personas). - #[test] - fn fallback_persona_model_set_provider_blank_uses_record_provider() { - let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic") - persona.provider = None; // blank provider on persona - - let snapshot = persona_snapshot_with_agent_config_fallback( - &persona, - Some("gpt-4o"), // record model (should be overridden by persona) - Some("openai"), // record provider (should be preserved) - ); - - assert_eq!( - snapshot.model.as_deref(), - Some("claude-opus-4"), - "persona model must win when persona has a value" - ); - assert_eq!( - snapshot.provider.as_deref(), - Some("openai"), - "record provider must be used when persona provider is blank" - ); - } - - /// Inverse: persona sets provider but not model → provider comes from - /// persona, model falls back to the record. - #[test] - fn fallback_persona_provider_set_model_blank_uses_record_model() { - let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic") - persona.model = None; // blank model on persona - - let snapshot = persona_snapshot_with_agent_config_fallback( - &persona, - Some("gpt-4o"), // record model (should be preserved) - Some("openai"), // record provider (should be overridden by persona) - ); - - assert_eq!( - snapshot.model.as_deref(), - Some("gpt-4o"), - "record model must be used when persona model is blank" - ); - assert_eq!( - snapshot.provider.as_deref(), - Some("anthropic"), - "persona provider must win when persona has a value" - ); - } -} +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs new file mode 100644 index 0000000000..87b37c3fb1 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -0,0 +1,785 @@ +use super::*; + +fn sample_persona() -> PersonaRecord { + PersonaRecord { + id: "test-persona".to_string(), + display_name: "Test Persona".to_string(), + avatar_url: Some("https://example.com/avatar.png".to_string()), + system_prompt: "You are a test assistant.".to_string(), + runtime: Some("goose".to_string()), + model: Some("claude-opus-4".to_string()), + provider: Some("anthropic".to_string()), + name_pool: vec!["Alpha".to_string(), "Beta".to_string()], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: Some("test-slug".to_string()), + env_vars: BTreeMap::from([("KEY".to_string(), "value".to_string())]), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + created_at: "2025-01-01T00:00:00Z".to_string(), + updated_at: "2025-01-01T00:00:00Z".to_string(), + } +} + +#[test] +fn monotonic_created_at_bumps_past_head() { + // No head: uses now (floor 0). + let now = nostr::Timestamp::now().as_secs() as i64; + let none = monotonic_created_at(None).as_secs() as i64; + assert!(none >= now, "no-head write must be >= now"); + + // Head in the FUTURE (same-second or clock-skewed): must bump to head+1, + // never reuse now (which would be <= head and lose the NIP-33 tiebreak). + let future_head = now + 1000; + let bumped = monotonic_created_at(Some(future_head)).as_secs() as i64; + assert_eq!( + bumped, + future_head + 1, + "must supersede a future head by +1" + ); + + // Head in the PAST: now already exceeds it, so now wins. + let past = monotonic_created_at(Some(now - 1000)).as_secs() as i64; + assert!(past >= now, "past head must not drag created_at backward"); +} + +#[test] +fn d_tag_uses_slug_when_available() { + let record = sample_persona(); + assert_eq!(persona_d_tag(&record), "test-slug"); +} + +#[test] +fn d_tag_falls_back_to_id() { + let mut record = sample_persona(); + record.source_team_persona_slug = None; + assert_eq!(persona_d_tag(&record), "test-persona"); +} + +/// Mirror of the relay slug grammar (`ingest.rs:923` `^[a-z0-9][a-z0-9_-]{0,63}$`) +/// so the normalization tests assert what the relay actually enforces. +fn passes_relay_slug_grammar(d: &str) -> bool { + let bytes = d.as_bytes(); + !d.is_empty() + && d.len() <= 64 + && (bytes[0].is_ascii_lowercase() || bytes[0].is_ascii_digit()) + && bytes[1..] + .iter() + .all(|&b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'_' || b == b'-') +} + +#[test] +fn d_tag_normalizes_pack_slug_to_relay_grammar() { + // The cited failing cases: mixed-case and leading-underscore pack slugs + // that the relay rejects un-normalized → pending forever. + for (raw, expected) in [ + ("CodeReviewer", "codereviewer"), + ("_ops", "a_ops"), + ("Code-Reviewer", "code-reviewer"), + ("UPPER_snake", "upper_snake"), + ("-leading-dash", "a-leading-dash"), + ] { + let mut record = sample_persona(); + record.source_team_persona_slug = Some(raw.to_string()); + let d = persona_d_tag(&record); + assert_eq!(d, expected, "normalization of {raw:?}"); + assert!( + passes_relay_slug_grammar(&d), + "normalized {raw:?} -> {d:?} still fails the relay grammar" + ); + } +} + +#[test] +fn d_tag_already_valid_slug_is_unchanged() { + // In-app personas use a lowercase-hex UUID id — already valid, must pass + // through untouched (no spurious coordinate change on existing data). + let mut record = sample_persona(); + record.source_team_persona_slug = None; + record.id = "11111111-2222-3333-4444-555555555555".to_string(); + let d = persona_d_tag(&record); + assert_eq!(d, "11111111-2222-3333-4444-555555555555"); + assert!(passes_relay_slug_grammar(&d)); +} + +#[test] +fn build_persona_event_produces_correct_kind() { + let record = sample_persona(); + let builder = build_persona_event(&record).unwrap(); + let keys = nostr::Keys::generate(); + let event = builder.sign_with_keys(&keys).unwrap(); + assert_eq!(event.kind.as_u16() as u32, KIND_PERSONA); +} + +#[test] +fn round_trip_serialization() { + let record = sample_persona(); + let builder = build_persona_event(&record).unwrap(); + let keys = nostr::Keys::generate(); + let event = builder.sign_with_keys(&keys).unwrap(); + + let restored = persona_from_event(&event).unwrap(); + assert_eq!(restored.id, "test-slug"); + assert_eq!(restored.display_name, "Test Persona"); + assert_eq!( + restored.avatar_url, + Some("https://example.com/avatar.png".to_string()) + ); + assert_eq!(restored.system_prompt, "You are a test assistant."); + assert_eq!(restored.runtime, Some("goose".to_string())); + assert_eq!(restored.model, Some("claude-opus-4".to_string())); + assert_eq!(restored.provider, Some("anthropic".to_string())); + assert_eq!(restored.name_pool, vec!["Alpha", "Beta"]); + // env_vars are not included in public persona events (secrets travel + // via NIP-44-encrypted engrams only). + assert!(restored.env_vars.is_empty()); + assert_eq!( + restored.source_team_persona_slug, + Some("test-slug".to_string()) + ); + assert!(!restored.is_builtin); + assert!(restored.is_active); +} + +/// NIP-AP reference vector (Event 1, `docs/nips/NIP-AP.md:195-207`): the +/// serialized content bytes MUST match the spec exactly, byte-for-byte. +/// serde emits fields in declaration order, so this pins the content +/// encoding — and therefore the NIP-01 event id — for cross-implementation +/// interop. The field order is `display_name, system_prompt, avatar_url, +/// runtime, model, provider, name_pool`. +#[test] +fn content_matches_nip_ap_vector() { + // Exact body from NIP-AP.md Event 1 (no trailing whitespace, no BOM). + const VECTOR: &str = r#"{"display_name":"Test Agent","system_prompt":"You are a test assistant.","avatar_url":"https://example.com/avatar.png","runtime":"goose","model":"claude-opus-4","provider":"anthropic","name_pool":["Alpha","Beta"]}"#; + + let content = PersonaEventContent { + display_name: "Test Agent".to_string(), + system_prompt: Some("You are a test assistant.".to_string()), + avatar_url: Some("https://example.com/avatar.png".to_string()), + runtime: Some("goose".to_string()), + model: Some("claude-opus-4".to_string()), + provider: Some("anthropic".to_string()), + name_pool: vec!["Alpha".to_string(), "Beta".to_string()], + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + }; + assert_eq!( + serde_json::to_string(&content).unwrap(), + VECTOR, + "serialized content drifted from the NIP-AP Event 1 vector" + ); + + // Hash invariance across the unified-model widening: REAL pre-revision + // content bytes (fixture string, not a round-trip through the new + // struct) must parse and re-serialize byte-identically, so + // persona_content_hash — the drift-badge basis — is unchanged on + // upgrade. A bare Option serializing "system_prompt":null would flip + // every persona's hash fleet-wide. + let parsed: PersonaEventContent = serde_json::from_str(VECTOR).unwrap(); + assert_eq!( + serde_json::to_string(&parsed).unwrap(), + VECTOR, + "pre-revision content bytes must survive a parse/serialize round-trip unchanged" + ); + assert_eq!( + persona_content_hash(&parsed), + { + use sha2::{Digest, Sha256}; + hex::encode(Sha256::digest(VECTOR.as_bytes())) + }, + "persona_content_hash of pre-revision bytes must equal the direct digest" + ); + + // Hash stability, adversarial shapes: the empty prompt and the + // minimal old-writer body (display_name + system_prompt only) are the + // two easiest regressions if the projection or skip attributes ever + // change. + const EMPTY_PROMPT: &str = r#"{"display_name":"X","system_prompt":""}"#; + let parsed: PersonaEventContent = serde_json::from_str(EMPTY_PROMPT).unwrap(); + assert_eq!(serde_json::to_string(&parsed).unwrap(), EMPTY_PROMPT); + const MINIMAL: &str = r#"{"display_name":"Minimal","system_prompt":"Hello."}"#; + let parsed: PersonaEventContent = serde_json::from_str(MINIMAL).unwrap(); + assert_eq!(serde_json::to_string(&parsed).unwrap(), MINIMAL); + + // An event built from this content carries the byte-exact vector as its + // signed content, so a second implementer following the spec computes + // the same NIP-01 id. + let record = PersonaRecord { + id: "test-agent".to_string(), + display_name: "Test Agent".to_string(), + avatar_url: Some("https://example.com/avatar.png".to_string()), + system_prompt: "You are a test assistant.".to_string(), + runtime: Some("goose".to_string()), + model: Some("claude-opus-4".to_string()), + provider: Some("anthropic".to_string()), + name_pool: vec!["Alpha".to_string(), "Beta".to_string()], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + created_at: "2025-01-01T00:00:00Z".to_string(), + updated_at: "2025-01-01T00:00:00Z".to_string(), + }; + let event = build_persona_event(&record) + .unwrap() + .sign_with_keys(&nostr::Keys::generate()) + .unwrap(); + assert_eq!(event.content, VECTOR); +} + +#[test] +fn round_trip_minimal_persona() { + let record = PersonaRecord { + id: "minimal".to_string(), + display_name: "Minimal".to_string(), + avatar_url: None, + system_prompt: "Hello".to_string(), + runtime: None, + model: None, + provider: None, + name_pool: vec![], + is_builtin: true, + is_active: false, + source_team: Some("team-1".to_string()), + source_team_persona_slug: None, + env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + created_at: "2025-01-01T00:00:00Z".to_string(), + updated_at: "2025-01-01T00:00:00Z".to_string(), + }; + + let builder = build_persona_event(&record).unwrap(); + let keys = nostr::Keys::generate(); + let event = builder.sign_with_keys(&keys).unwrap(); + + let restored = persona_from_event(&event).unwrap(); + assert_eq!(restored.id, "minimal"); + assert_eq!(restored.display_name, "Minimal"); + assert_eq!(restored.avatar_url, None); + assert_eq!(restored.runtime, None); + assert_eq!(restored.model, None); + assert_eq!(restored.provider, None); + assert!(restored.name_pool.is_empty()); + assert!(restored.env_vars.is_empty()); + // Deserialized persona is always non-builtin and active + assert!(!restored.is_builtin); + assert!(restored.is_active); +} + +#[test] +fn build_persona_delete_has_single_a_tag_no_e_tag() { + const OWNER: &str = "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798"; + let builder = build_persona_delete("test-slug", OWNER).unwrap(); + let keys = nostr::Keys::generate(); + let event = builder.sign_with_keys(&keys).unwrap(); + + assert_eq!(event.kind, Kind::Custom(5)); + + let a_tags: Vec<&[String]> = event + .tags + .iter() + .map(|t| t.as_slice()) + .filter(|v| v.first().map(String::as_str) == Some("a")) + .collect(); + assert_eq!(a_tags.len(), 1); + assert_eq!(a_tags[0][1], format!("{KIND_PERSONA}:{OWNER}:test-slug")); + + // An e-tag would route to the event-id deletion path and leave the + // replaceable coordinate live — the tombstone must carry none. + assert!(event + .tags + .iter() + .all(|t| t.as_slice().first().map(String::as_str) != Some("e"))); +} + +/// NIP-AP behavioral defaults are LIVE since B5 (create-path +/// unification): the wire fields are carried on PersonaRecord in wire +/// shape and re-emitted verbatim by the projection — a foreign +/// definition's behavioral values now survive a local +/// edit-and-republish cycle. This test replaces +/// `behavioral_defaults_are_staged_not_applied` (the staging lock), +/// whose deliberate removal was pinned in the B5 review gates. +#[test] +fn behavioral_defaults_survive_record_round_trip() { + const FOREIGN: &str = r#"{"display_name":"F","system_prompt":"p","respond_to":"anyone","respond_to_allowlist":["deadbeef"],"mcp_toolsets":"default","parallelism":4}"#; + let parsed: PersonaEventContent = serde_json::from_str(FOREIGN).unwrap(); + // Wire layer preserves the fields... + assert_eq!(parsed.respond_to.as_deref(), Some("anyone")); + assert_eq!(parsed.parallelism, Some(4)); + // ...and the record round-trip now carries them through. + let record = persona_from_event_content_for_test(parsed); + let reprojected = persona_event_content(&record); + assert_eq!(reprojected.respond_to.as_deref(), Some("anyone")); + assert_eq!(reprojected.respond_to_allowlist, vec!["deadbeef"]); + assert_eq!(reprojected.mcp_toolsets.as_deref(), Some("default")); + assert_eq!(reprojected.parallelism, Some(4)); +} + +/// B5 hash row 1: a quad-absent definition's content bytes — and +/// therefore `persona_content_hash` — are identical before and after +/// quad activation. Pre-activation the projection hardcoded `None`; +/// post-activation it copies the record's (absent) quad. Both serialize +/// to the same bytes via `skip_serializing_if`, so no drift badge flips +/// and no republish wave fires for quad-absent definitions. +#[test] +fn quad_absent_definition_hash_stable_across_activation() { + let record = PersonaRecord { + id: "quad-absent".to_string(), + display_name: "Test".to_string(), + avatar_url: None, + system_prompt: "Hello".to_string(), + runtime: Some("goose".to_string()), + model: Some("gpt-oss".to_string()), + provider: None, + name_pool: vec!["nib".to_string()], + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + }; + let live = persona_event_content(&record); + // The reserved-era projection: identical fields, quad hardcoded off. + let reserved_era = PersonaEventContent { + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + ..live.clone() + }; + assert_eq!( + serde_json::to_string(&live).unwrap(), + serde_json::to_string(&reserved_era).unwrap(), + "quad-absent projection must serialize byte-identically to the reserved era" + ); + assert_eq!( + persona_content_hash(&live), + persona_content_hash(&reserved_era) + ); +} + +/// Test-only bridge: build a PersonaRecord from parsed content the same +/// way `persona_from_event` maps fields, without needing a signed event. +fn persona_from_event_content_for_test(content: PersonaEventContent) -> PersonaRecord { + PersonaRecord { + id: "staged".to_string(), + display_name: content.display_name, + avatar_url: content.avatar_url, + system_prompt: content.system_prompt.unwrap_or_default(), + runtime: content.runtime, + model: content.model, + provider: content.provider, + name_pool: content.name_pool, + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: BTreeMap::new(), + respond_to: content.respond_to, + respond_to_allowlist: content.respond_to_allowlist, + mcp_toolsets: content.mcp_toolsets, + parallelism: content.parallelism, + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + } +} + +#[test] +fn persona_content_hash_is_deterministic() { + let content = PersonaEventContent { + display_name: "Test".to_string(), + avatar_url: None, + system_prompt: Some("Hello".to_string()), + runtime: None, + model: None, + provider: None, + name_pool: vec![], + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + }; + let hash1 = persona_content_hash(&content); + let hash2 = persona_content_hash(&content); + assert_eq!(hash1, hash2); + assert_eq!(hash1.len(), 64); // SHA-256 hex +} + +#[test] +fn persona_content_hash_changes_on_edit() { + let content1 = PersonaEventContent { + display_name: "Test".to_string(), + avatar_url: None, + system_prompt: Some("Hello".to_string()), + runtime: None, + model: None, + provider: None, + name_pool: vec![], + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + }; + let mut content2 = content1.clone(); + content2.system_prompt = Some("Goodbye".to_string()); + assert_ne!( + persona_content_hash(&content1), + persona_content_hash(&content2) + ); +} + +// ── persona_field_with_record_fallback ──────────────────────────────────── + +#[test] +fn field_fallback_persona_present_wins() { + assert_eq!( + persona_field_with_record_fallback(Some("persona-model"), Some("record-model")), + Some("persona-model".to_owned()), + ); +} + +#[test] +fn field_fallback_persona_blank_uses_record() { + assert_eq!( + persona_field_with_record_fallback(None, Some("record-model")), + Some("record-model".to_owned()), + ); + assert_eq!( + persona_field_with_record_fallback(Some(" "), Some("record-model")), + Some("record-model".to_owned()), + ); +} + +#[test] +fn field_fallback_both_blank_is_none() { + assert_eq!(persona_field_with_record_fallback(None, None), None); + assert_eq!(persona_field_with_record_fallback(Some(""), Some("")), None); +} + +#[test] +fn field_fallback_record_blank_is_none() { + assert_eq!( + persona_field_with_record_fallback(None, Some(" ")), + None, + "whitespace-only record value must also be treated as blank" + ); +} + +// ── persona_snapshot_with_agent_config_fallback ──────────────────────────── + +/// Helper: a persona with no model/provider configured. +fn blank_model_persona() -> PersonaRecord { + PersonaRecord { + model: None, + provider: None, + ..sample_persona() + } +} + +/// (a) Persona leaves model/provider blank, agent record has values → +/// record values preserved AND source_version still updated to current hash. +#[test] +fn fallback_preserves_record_values_when_persona_blank() { + let persona = blank_model_persona(); + let expected_version = persona_content_hash(&persona_event_content(&persona)); + + let snapshot = + persona_snapshot_with_agent_config_fallback(&persona, Some("gpt-4o"), Some("openai")); + + assert_eq!( + snapshot.model.as_deref(), + Some("gpt-4o"), + "blank persona model must fall back to agent record value" + ); + assert_eq!( + snapshot.provider.as_deref(), + Some("openai"), + "blank persona provider must fall back to agent record value" + ); + assert_eq!( + snapshot.source_version, expected_version, + "source_version must still reflect current persona hash" + ); +} + +/// (b) Persona has model/provider set → persona wins over agent record. +#[test] +fn fallback_persona_wins_when_set() { + let persona = sample_persona(); // has model=Some("claude-opus-4"), provider=Some("anthropic") + + let snapshot = persona_snapshot_with_agent_config_fallback( + &persona, + Some("gpt-4o"), // agent had a different model + Some("openai"), // agent had a different provider + ); + + assert_eq!( + snapshot.model.as_deref(), + Some("claude-opus-4"), + "persona model must win when persona has a value" + ); + assert_eq!( + snapshot.provider.as_deref(), + Some("anthropic"), + "persona provider must win when persona has a value" + ); +} + +/// (c) Both blank → snapshot keeps None; a genuinely unconfigured agent +/// stays unconfigured (no fabricated values). +#[test] +fn fallback_both_blank_stays_none() { + let persona = blank_model_persona(); + + let snapshot = persona_snapshot_with_agent_config_fallback( + &persona, None, // agent also has no model + None, // agent also has no provider + ); + + assert!( + snapshot.model.is_none(), + "neither persona nor agent has model — snapshot must be None" + ); + assert!( + snapshot.provider.is_none(), + "neither persona nor agent has provider — snapshot must be None" + ); +} + +/// Whitespace-only values on the persona are treated as blank; agent +/// fallback applies. +#[test] +fn fallback_treats_whitespace_only_persona_value_as_blank() { + let mut persona = sample_persona(); + persona.model = Some(" ".to_string()); + persona.provider = Some("\t".to_string()); + + let snapshot = persona_snapshot_with_agent_config_fallback( + &persona, + Some("claude-opus-4"), + Some("anthropic"), + ); + + assert_eq!( + snapshot.model.as_deref(), + Some("claude-opus-4"), + "whitespace-only persona model must be treated as blank" + ); + assert_eq!( + snapshot.provider.as_deref(), + Some("anthropic"), + "whitespace-only persona provider must be treated as blank" + ); +} + +/// Cross-field independence: persona sets model but not provider → model +/// comes from persona, provider falls back to the record. This is the +/// practically common case (model-only personas). +#[test] +fn fallback_persona_model_set_provider_blank_uses_record_provider() { + let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic") + persona.provider = None; // blank provider on persona + + let snapshot = persona_snapshot_with_agent_config_fallback( + &persona, + Some("gpt-4o"), // record model (should be overridden by persona) + Some("openai"), // record provider (should be preserved) + ); + + assert_eq!( + snapshot.model.as_deref(), + Some("claude-opus-4"), + "persona model must win when persona has a value" + ); + assert_eq!( + snapshot.provider.as_deref(), + Some("openai"), + "record provider must be used when persona provider is blank" + ); +} + +/// Inverse: persona sets provider but not model → provider comes from +/// persona, model falls back to the record. +#[test] +fn fallback_persona_provider_set_model_blank_uses_record_model() { + let mut persona = sample_persona(); // model=Some("claude-opus-4"), provider=Some("anthropic") + persona.model = None; // blank model on persona + + let snapshot = persona_snapshot_with_agent_config_fallback( + &persona, + Some("gpt-4o"), // record model (should be preserved) + Some("openai"), // record provider (should be overridden by persona) + ); + + assert_eq!( + snapshot.model.as_deref(), + Some("gpt-4o"), + "record model must be used when persona model is blank" + ); + assert_eq!( + snapshot.provider.as_deref(), + Some("anthropic"), + "persona provider must win when persona has a value" + ); +} + +// Gated off Windows for the same reason as `archive::real_relay`: +// `build_app_state()` pulls native DLLs unavailable in the Windows CI +// runner. This stub-relay test is hermetic (localhost axum) otherwise. +#[cfg(not(target_os = "windows"))] +mod flush_barrier { + use super::*; + use crate::app_state::build_app_state; + use crate::managed_agents::retention::{ + get_retained_event, open_retention_db, retain_event, tombstone_retention_d_tag, + RetainedEvent, + }; + use nostr::JsonUtil; + + /// Stub relay: `POST /events` rejects kind:5 with HTTP 500, accepts + /// everything else. Returns the HTTP base URL. + async fn spawn_stub_relay() -> String { + use axum::{http::StatusCode, routing::post, Router}; + + let app = Router::new().route( + "/events", + post(|body: String| async move { + let event: serde_json::Value = serde_json::from_str(&body).unwrap_or_default(); + if event.get("kind").and_then(serde_json::Value::as_u64) == Some(5) { + return (StatusCode::INTERNAL_SERVER_ERROR, String::new()); + } + ( + StatusCode::OK, + serde_json::json!({ + "event_id": event.get("id").and_then(serde_json::Value::as_str).unwrap_or(""), + "accepted": true, + "message": "" + }) + .to_string(), + ) + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind stub relay"); + let addr = listener.local_addr().expect("stub relay addr"); + tokio::spawn(async move { + axum::serve(listener, app).await.ok(); + }); + format!("http://{addr}") + } + + fn retain_signed( + conn: &rusqlite::Connection, + keys: &nostr::Keys, + kind: u32, + retention_d_tag: &str, + builder: nostr::EventBuilder, + created_at: i64, + ) { + let event = builder.sign_with_keys(keys).expect("sign test event"); + retain_event( + conn, + &RetainedEvent { + kind, + pubkey: keys.public_key().to_hex(), + d_tag: retention_d_tag.to_string(), + content: event.content.to_string(), + created_at, + raw_event: event.as_json(), + pending_sync: true, + }, + ) + .expect("retain test event"); + } + + /// The mid-sweep barrier: a tombstone the relay rejects must defer its + /// own replacement to the next sweep (still pending, not counted as + /// flushed) while unrelated rows in the same sweep publish normally. + /// Failing toward stay-deleted is the safe direction — the deferred + /// replacement can never be wiped by its own late tombstone. + #[tokio::test] + async fn failed_tombstone_defers_replacement_within_sweep() { + let keys = nostr::Keys::generate(); + let pubkey = keys.public_key().to_hex(); + let dir = tempfile::tempdir().expect("tempdir"); + let db_path = dir.path().join("retention.db"); + + { + let conn = open_retention_db(&db_path).expect("open db"); + // Tombstone (publishes first, relay rejects it). + retain_signed( + &conn, + &keys, + 5, + &tombstone_retention_d_tag(KIND_PERSONA, "covered"), + build_persona_delete("covered", &pubkey).unwrap(), + 1000, + ); + // Its replacement at the same coordinate (must defer). + retain_signed( + &conn, + &keys, + KIND_PERSONA, + "covered", + EventBuilder::new(Kind::Custom(KIND_PERSONA as u16), "{}") + .tags(vec![Tag::parse(["d", "covered"]).unwrap()]), + 2000, + ); + // Unrelated coordinate (must publish despite the barrier). + retain_signed( + &conn, + &keys, + KIND_PERSONA, + "unrelated", + EventBuilder::new(Kind::Custom(KIND_PERSONA as u16), "{}") + .tags(vec![Tag::parse(["d", "unrelated"]).unwrap()]), + 1500, + ); + } + + let state = build_app_state(); + *state.relay_url_override.lock().unwrap() = Some(spawn_stub_relay().await); + + let flushed = flush_pending_events(&db_path, &state).await.expect("flush"); + assert_eq!(flushed, 1, "only the unrelated row publishes"); + + let conn = open_retention_db(&db_path).expect("reopen db"); + let row = |kind: u32, d_tag: &str| { + get_retained_event(&conn, kind, &pubkey, d_tag) + .unwrap() + .unwrap() + }; + assert!( + row(5, &tombstone_retention_d_tag(KIND_PERSONA, "covered")).pending_sync, + "failed tombstone stays pending" + ); + assert!( + row(KIND_PERSONA, "covered").pending_sync, + "deferred replacement stays pending" + ); + assert!( + !row(KIND_PERSONA, "unrelated").pending_sync, + "unrelated row marked synced" + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/personas.rs b/desktop/src-tauri/src/managed_agents/personas.rs index 3e88661580..ccaf4f31f1 100644 --- a/desktop/src-tauri/src/managed_agents/personas.rs +++ b/desktop/src-tauri/src/managed_agents/personas.rs @@ -237,6 +237,10 @@ fn built_in_persona_records(now: &str) -> Vec { source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: now.to_string(), updated_at: now.to_string(), }) diff --git a/desktop/src-tauri/src/managed_agents/personas/tests.rs b/desktop/src-tauri/src/managed_agents/personas/tests.rs index b4e5764dd4..0c10d6e765 100644 --- a/desktop/src-tauri/src/managed_agents/personas/tests.rs +++ b/desktop/src-tauri/src/managed_agents/personas/tests.rs @@ -22,6 +22,10 @@ fn custom_persona(id: &str, display_name: &str) -> PersonaRecord { source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2026-03-19T00:00:00Z".to_string(), updated_at: "2026-03-19T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c3904c238f..aff4bf17ae 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -1179,6 +1179,10 @@ mod tests { is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, }; diff --git a/desktop/src-tauri/src/managed_agents/retention.rs b/desktop/src-tauri/src/managed_agents/retention.rs index 1ba5d14a5e..1c66c8b7b6 100644 --- a/desktop/src-tauri/src/managed_agents/retention.rs +++ b/desktop/src-tauri/src/managed_agents/retention.rs @@ -64,6 +64,26 @@ pub fn tombstone_retention_d_tag(target_kind: u32, d_tag: &str) -> String { format!("{target_kind}:{d_tag}") } +/// Whether a pending row must be deferred to the next sweep because a kind:5 +/// tombstone covering its coordinate failed to publish earlier in the same +/// sweep. +/// +/// `get_pending_sync` orders tombstones first so a deletion always reaches the +/// relay before the replacement that supersedes it — but the flush is +/// best-effort per row, so a tombstone that fails mid-sweep would otherwise be +/// leapfrogged by its own replacement and then wipe it on the next sweep. +/// Deferring the replacement restores the ordering guarantee: next sweep the +/// `ORDER BY` puts the tombstone first again. Kind:5 rows are never deferred. +pub fn deferred_behind_failed_tombstone( + kind: u32, + pubkey: &str, + d_tag: &str, + failed_tombstones: &std::collections::HashSet<(String, String)>, +) -> bool { + kind != 5 + && failed_tombstones.contains(&(pubkey.to_string(), tombstone_retention_d_tag(kind, d_tag))) +} + /// Upsert a persona event into the retention store. /// /// Only replaces if the new event has a newer or equal `created_at` (NIP-33 semantics). @@ -198,12 +218,20 @@ pub fn get_retained_personas( } /// Get all events marked as pending sync (not yet confirmed on relay). +/// +/// Tombstones (kind:5) sort FIRST: a delete retained in one session and its +/// coordinate's replacement retained in a later one (B5 backfill resurrecting +/// a deleted definition) must publish in that order — the relay's a-tag +/// deletion soft-deletes every live row at the coordinate with no timestamp +/// comparison, so a tombstone published AFTER the replacement would wipe it. +/// Within each group, oldest first for the same reason. pub fn get_pending_sync(conn: &Connection) -> Result, String> { let mut stmt = conn .prepare( "SELECT kind, pubkey, d_tag, content, created_at, raw_event, pending_sync FROM persona_events - WHERE pending_sync = 1", + WHERE pending_sync = 1 + ORDER BY (kind != 5), created_at ASC", ) .map_err(|e| format!("failed to prepare pending sync query: {e}"))?; @@ -630,4 +658,84 @@ mod tests { assert_eq!(row.created_at, 2000); assert!(!row.content.contains("Stale")); } + + #[test] + fn pending_sync_publishes_tombstones_before_replacements() { + // B5 resurrection race: a kind:5 retained in session N and the same + // coordinate's replacement 30175 retained on the next boot can sit + // pending together. The relay's a-tag deletion ignores timestamps, + // so the tombstone MUST publish first or it wipes the replacement. + let conn = test_db(); + let replacement = RetainedEvent { + kind: 30175, + created_at: 2000, + pending_sync: true, + ..sample_event() + }; + retain_event(&conn, &replacement).unwrap(); + let tombstone = RetainedEvent { + kind: 5, + d_tag: tombstone_retention_d_tag(30175, "test-persona"), + content: String::new(), + created_at: 1000, + pending_sync: true, + ..sample_event() + }; + retain_event(&conn, &tombstone).unwrap(); + + let pending = get_pending_sync(&conn).unwrap(); + assert_eq!(pending.len(), 2); + assert_eq!(pending[0].kind, 5, "tombstone first"); + assert_eq!(pending[1].kind, 30175, "replacement second"); + } + + #[test] + fn deferral_predicate_is_kind_and_pubkey_qualified() { + // Mid-sweep barrier semantics: a failed tombstone defers ONLY the + // replacement at its exact coordinate — same target kind, same pubkey. + use std::collections::HashSet; + + let failed: HashSet<(String, String)> = HashSet::from([( + "abc123".to_string(), + tombstone_retention_d_tag(30175, "test-persona"), + )]); + + // The covered replacement defers. + assert!(deferred_behind_failed_tombstone( + 30175, + "abc123", + "test-persona", + &failed + )); + // Kind-qualified: a coinciding slug under a DIFFERENT kind is a + // distinct coordinate (the cross-kind collision the retention d-tag + // encoding exists to prevent) — never deferred. + assert!(!deferred_behind_failed_tombstone( + 30177, + "abc123", + "test-persona", + &failed + )); + // Never crosses pubkeys. + assert!(!deferred_behind_failed_tombstone( + 30175, + "other-key", + "test-persona", + &failed + )); + // Never defers kind:5 rows, even at a "matching" retention key. + assert!(!deferred_behind_failed_tombstone( + 5, + "abc123", + "test-persona", + &failed + )); + // Unrelated d-tags publish normally. + assert!(!deferred_behind_failed_tombstone( + 30175, + "abc123", + "other-persona", + &failed + )); + } } diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index b5ffb9c8b1..354e343070 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1732,7 +1732,11 @@ pub fn spawn_agent_child( // persona) is what keeps a running agent pinned across restarts: a persona // edit reaches the agent only via delete+respawn, which rewrites the // snapshot. - let effective_prompt = record.system_prompt.clone(); + // Prompt via the shared spawn-effective filter — the SAME function the + // config hash digests, so env write and badge cannot disagree (see + // `effective_spawn_prompt` for the Some("")/None collapse and the + // team-pack suppression exception). + let effective_prompt = super::spawn_hash::effective_spawn_prompt(record); let effective_model = record.model.clone(); let effective_provider = record.provider.clone(); diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 626ceb4379..bfb7e1cbd2 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -170,6 +170,10 @@ fn fixture( is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, } } @@ -287,6 +291,10 @@ fn persona_with_provider( source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2026-06-09T00:00:00Z".to_string(), updated_at: "2026-06-09T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index f17d9eea6c..e369fd443e 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -33,6 +33,25 @@ use super::{ types::{ManagedAgentRecord, PersonaRecord}, }; +/// The prompt a spawn would actually deliver: `Some("")` collapses to `None` +/// because an empty BUZZ_ACP_SYSTEM_PROMPT is no prompt — EXCEPT for +/// team-pack records, where buzz-acp falls back to the pack persona's prompt +/// when the env var is absent, making set-but-empty a deliberate suppression. +/// +/// The single source of truth for the spawn env write AND the config hash: +/// both call this, so they cannot disagree (the hash's contract is "digest +/// what a spawn would actually run"). B5 hash row 2 depends on the collapse: +/// backfilled prompt-less records re-snapshot to `Some("")` from their +/// manufactured definition and must not trip the restart badge. +pub(crate) fn effective_spawn_prompt(record: &ManagedAgentRecord) -> Option { + let has_pack_fallback = + record.persona_team_dir.is_some() && record.persona_name_in_team.is_some(); + record + .system_prompt + .clone() + .filter(|p| has_pack_fallback || !p.is_empty()) +} + /// Digest the effective spawn configuration of `record` under the current /// `personas`, resolving a blank record relay against `workspace_relay`. /// Pure — no `AppHandle`, no disk, no keyring. @@ -91,7 +110,9 @@ pub(crate) fn spawn_config_hash( // resolved: a blank record relay spawns on the workspace relay, so a // workspace relay change must trip the badge. crate::relay::effective_agent_relay_url(&record.relay_url, workspace_relay).hash(&mut hasher); - record.system_prompt.hash(&mut hasher); + // Prompt via the shared spawn-effective filter (see its doc for the + // Some("")/None collapse and the team-pack exception). + effective_spawn_prompt(record).hash(&mut hasher); record.model.hash(&mut hasher); record.provider.hash(&mut hasher); record.auth_tag.hash(&mut hasher); diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index 7ff4fe0e25..a524457f9e 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -51,6 +51,10 @@ fn record() -> ManagedAgentRecord { is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, } } @@ -70,6 +74,10 @@ fn persona(id: &str, runtime: Option<&str>, prompt: &str) -> PersonaRecord { source_team: None, source_team_persona_slug: None, env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "now".into(), updated_at: "now".into(), } @@ -302,3 +310,92 @@ fn non_spawn_bookkeeping_fields_do_not_change_hash() { spawn_config_hash(&edited, &[], "wss://ws.example") ); } + +#[test] +fn resnapshot_does_not_clobber_record_quad_with_definition_absent_quad() { + // B5 hash row 3: the prospective re-snapshot copies ONLY + // prompt/model/provider/env from the linked definition. An instance + // whose owner hand-set respond_to/allowlist/parallelism/toolsets must + // hash identically whether or not its definition carries a quad — + // activation of the definition-level defaults must never reach through + // spawn and overwrite instance state. + let quadless_definition = vec![persona("p1", Some("goose"), "Persona prompt.")]; + + let mut rec = record(); + rec.persona_id = Some("p1".into()); + rec.respond_to = RespondTo::Allowlist; + rec.respond_to_allowlist = vec!["a".repeat(64)]; + rec.parallelism = 4; + rec.mcp_toolsets = Some("default,canvas".into()); + + let mut definition_with_quad = quadless_definition.clone(); + definition_with_quad[0].respond_to = Some("anyone".into()); + definition_with_quad[0].parallelism = Some(8); + definition_with_quad[0].mcp_toolsets = Some("default".into()); + + assert_eq!( + spawn_config_hash(&rec, &quadless_definition, "wss://ws.example"), + spawn_config_hash(&rec, &definition_with_quad, "wss://ws.example"), + "definition quad must not leak into the spawn hash of an existing instance" + ); +} + +#[test] +fn empty_prompt_hashes_like_absent_prompt() { + // B5 hash row 2 foundation: Some("") and None spawn identically (env var + // absent either way), so they must hash equal — a backfilled prompt-less + // record re-snapshots to Some("") and must not trip the badge. + let mut absent = record(); + absent.system_prompt = None; + let mut empty = record(); + empty.system_prompt = Some(String::new()); + assert_eq!( + spawn_config_hash(&absent, &[], "wss://ws.example"), + spawn_config_hash(&empty, &[], "wss://ws.example"), + ); +} + +#[test] +fn team_pack_records_keep_empty_vs_absent_prompt_distinction() { + // Wrinkle-1 exception (Pinky): with BUZZ_ACP_PERSONA_PACK set, buzz-acp + // inherits the pack persona's prompt when the env var is ABSENT, while + // set-but-empty suppresses it. For team records the two states spawn + // differently, so they must hash differently. + let mut absent = record(); + absent.persona_team_dir = Some(std::path::PathBuf::from("/teams/alpha")); + absent.persona_name_in_team = Some("lep".into()); + absent.system_prompt = None; + + let mut empty = absent.clone(); + empty.system_prompt = Some(String::new()); + + assert_ne!( + spawn_config_hash(&absent, &[], "wss://ws.example"), + spawn_config_hash(&empty, &[], "wss://ws.example"), + "suppressed pack prompt is a different spawn than inherited pack prompt" + ); +} + +#[test] +fn effective_spawn_prompt_matches_hash_semantics() { + // The env write and the hash share effective_spawn_prompt — this row + // pins the helper's own semantics so a refactor of either caller cannot + // silently diverge from the contract. + let mut r = record(); + r.system_prompt = Some(String::new()); + assert_eq!( + effective_spawn_prompt(&r), + None, + "empty collapses to absent" + ); + r.system_prompt = Some("real".into()); + assert_eq!(effective_spawn_prompt(&r).as_deref(), Some("real")); + r.system_prompt = Some(String::new()); + r.persona_team_dir = Some(std::path::PathBuf::from("/teams/alpha")); + r.persona_name_in_team = Some("lep".into()); + assert_eq!( + effective_spawn_prompt(&r).as_deref(), + Some(""), + "team-pack set-but-empty survives (deliberate suppression)" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/team_repair.rs b/desktop/src-tauri/src/managed_agents/team_repair.rs index 7777cb8722..88ecc2e852 100644 --- a/desktop/src-tauri/src/managed_agents/team_repair.rs +++ b/desktop/src-tauri/src/managed_agents/team_repair.rs @@ -249,6 +249,10 @@ mod tests { source_team: source_team.map(|s| s.to_string()), source_team_persona_slug: slug.map(|s| s.to_string()), env_vars: Default::default(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2025-01-01T00:00:00Z".to_string(), updated_at: "2025-01-01T00:00:00Z".to_string(), } @@ -319,6 +323,10 @@ mod tests { is_active: true, source_team: None, source_team_persona_slug: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_mcp_toolsets: None, + definition_parallelism: None, relay_mesh: None, } } diff --git a/desktop/src-tauri/src/managed_agents/teams.rs b/desktop/src-tauri/src/managed_agents/teams.rs index 33de8c7f45..25ec767430 100644 --- a/desktop/src-tauri/src/managed_agents/teams.rs +++ b/desktop/src-tauri/src/managed_agents/teams.rs @@ -295,6 +295,10 @@ pub fn import_team_from_directory( env_vars: crate::managed_agents::env_vars::filter_derived_provider_model_env_vars( p.runtime_env_vars.iter().cloned(), ), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: now.clone(), updated_at: now.clone(), }) @@ -530,6 +534,10 @@ pub fn sync_team_from_dir( env_vars: crate::managed_agents::env_vars::filter_derived_provider_model_env_vars( dir_persona.runtime_env_vars.iter().cloned(), ), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: now.clone(), updated_at: now.clone(), }; @@ -784,6 +792,10 @@ mod tests { source_team: None, source_team_persona_slug: None, env_vars: std::collections::BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2026-03-20T00:00:00Z".to_string(), updated_at: "2026-03-20T00:00:00Z".to_string(), } diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 33e715a207..28a734e6e3 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -63,6 +63,19 @@ pub struct PersonaRecord { /// Stored as a BTreeMap for deterministic on-disk ordering. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub env_vars: BTreeMap, + /// NIP-AP behavioral defaults, stored in WIRE shape (kebab-case string, + /// not the `RespondTo` enum) so `persona_event_content` is a verbatim + /// copy and quad-absent records serialize byte-identically to the + /// pre-activation era. Copied onto instances at mint time only — spawn + /// re-snapshot never touches them. Validated at the instance boundary. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub respond_to: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub respond_to_allowlist: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub mcp_toolsets: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub parallelism: Option, pub created_at: String, pub updated_at: String, } @@ -121,6 +134,10 @@ impl PersonaRecord { is_active: self.is_active, source_team: self.source_team, source_team_persona_slug: self.source_team_persona_slug, + definition_respond_to: self.respond_to, + definition_respond_to_allowlist: self.respond_to_allowlist, + definition_mcp_toolsets: self.mcp_toolsets, + definition_parallelism: self.parallelism, relay_mesh: None, } } @@ -150,6 +167,10 @@ impl ManagedAgentRecord { source_team: self.source_team.clone(), source_team_persona_slug: self.source_team_persona_slug.clone(), env_vars: self.env_vars.clone(), + respond_to: self.definition_respond_to.clone(), + respond_to_allowlist: self.definition_respond_to_allowlist.clone(), + mcp_toolsets: self.definition_mcp_toolsets.clone(), + parallelism: self.definition_parallelism, created_at: self.created_at.clone(), updated_at: self.updated_at.clone(), }) @@ -337,6 +358,22 @@ pub struct ManagedAgentRecord { /// definition's slug within its source team. #[serde(default, skip_serializing_if = "Option::is_none")] pub source_team_persona_slug: Option, + /// NIP-AP definition-level behavioral defaults, absorbed from + /// `PersonaRecord` in WIRE shape (kebab-case string / optional u32), + /// distinct from the instance-side `respond_to`/`respond_to_allowlist`/ + /// `parallelism` fields above: these are what a *definition* advertises + /// and are copied onto instances at mint time only. Wire shape (not the + /// `RespondTo` enum) so absent-ness and unknown future mode strings + /// round-trip byte-identically through the store — parsed/validated + /// solely at the mint boundary. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub definition_respond_to: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub definition_respond_to_allowlist: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub definition_mcp_toolsets: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub definition_parallelism: Option, /// Typed marker for relay-mesh agents. `Some(_)` means this agent runs its /// inference through Buzz's relay-mesh local endpoint; the `model_ref` is /// the served model id to route to. `None` is a normal agent. @@ -487,8 +524,12 @@ pub struct CreateManagedAgentRequest { pub start_on_app_launch: bool, #[serde(default)] pub backend: BackendKind, + /// `None` = caller expressed no preference: the definition's + /// `respond_to` default applies when linked, `RespondTo::default()` + /// otherwise. `Some` is an explicit instance-level choice and always + /// wins over the definition default. #[serde(default)] - pub respond_to: RespondTo, + pub respond_to: Option, /// Raw allowlist as received from the frontend. Validated and normalized /// before being written to the record. #[serde(default)] @@ -505,50 +546,6 @@ pub struct CreateManagedAgentResponse { pub spawn_error: Option, } -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct CreatePersonaRequest { - pub display_name: String, - pub avatar_url: Option, - pub system_prompt: String, - #[serde(default)] - pub runtime: Option, - #[serde(default)] - pub model: Option, - #[serde(default)] - pub provider: Option, - #[serde(default)] - pub name_pool: Vec, - /// Environment variables for agents created from this persona. - #[serde(default)] - pub env_vars: BTreeMap, -} - -#[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct UpdatePersonaRequest { - pub id: String, - pub display_name: String, - pub avatar_url: Option, - pub system_prompt: String, - #[serde(default)] - pub runtime: Option, - #[serde(default)] - pub model: Option, - #[serde(default)] - pub provider: Option, - #[serde(default)] - pub name_pool: Vec, - /// Environment variables for agents created from this persona. - /// - /// Absent (`None`) = don't touch the stored value (caller didn't include - /// the field). `Some(map)` = replace entirely (empty map clears all). - /// Defaulting an omitted field to an empty map would silently erase - /// stored credentials when an unrelated field is edited. - #[serde(default)] - pub env_vars: Option>, -} - #[derive(Debug, Serialize)] pub struct ManagedAgentLogResponse { pub content: String, @@ -828,6 +825,22 @@ impl RespondTo { Self::Anyone => "anyone", } } + + /// Parse the NIP-AP wire string. Definitions carry `respond_to` as + /// opaque data everywhere else; this is the single parse boundary + /// (instance mint), and an unrecognized mode fails LOUDLY here rather + /// than silently defaulting — a typo'd definition must not mint an + /// agent with a different audience than its author intended. + pub fn parse_wire(value: &str) -> Result { + match value { + "owner-only" => Ok(Self::OwnerOnly), + "allowlist" => Ok(Self::Allowlist), + "anyone" => Ok(Self::Anyone), + other => Err(format!( + "definition respond_to '{other}' is not a recognized mode (expected 'owner-only', 'allowlist', or 'anyone')" + )), + } + } } /// Validate and normalize a respond-to allowlist. @@ -857,5 +870,98 @@ pub fn validate_respond_to_allowlist(input: &[String]) -> Result, St Ok(out) } +/// The behavioral fields resolved for a new instance at mint time. +#[derive(Debug, PartialEq, Eq)] +pub struct MintBehavioralDefaults { + pub respond_to: RespondTo, + pub respond_to_allowlist: Vec, + pub mcp_toolsets: Option, + /// Validated (1..=32) when present; caller applies its own default. + pub parallelism: Option, +} + +/// Resolve the NIP-AP behavioral quad for a new instance: explicit input +/// wins, then the linked definition's defaults, then client defaults. +/// +/// This is the ONLY place definition behavioral strings are parsed — an +/// unrecognized `respond_to` mode or out-of-range `parallelism` on a +/// definition fails the mint loudly instead of silently substituting a +/// default the definition author did not choose. The empty-allowlist guard +/// fires here too, because inbound definitions bypass the dialog entirely. +/// +/// `input_allowlist` must already be normalized via +/// [`validate_respond_to_allowlist`]; the definition's allowlist is +/// validated here since it arrives from the wire. +pub fn resolve_mint_behavioral_defaults( + input_respond_to: Option, + input_allowlist: Vec, + input_mcp_toolsets: Option, + input_parallelism: Option, + definition: Option<&PersonaRecord>, +) -> Result { + let (respond_to, respond_to_allowlist) = match input_respond_to { + // Explicit instance-level choice: the definition default is ignored + // wholesale (mode AND list travel together). + Some(mode) => (mode, input_allowlist), + None => match definition.and_then(|d| d.respond_to.as_deref()) { + Some(wire) => { + let mode = RespondTo::parse_wire(wire)?; + let list = if input_allowlist.is_empty() { + validate_respond_to_allowlist( + definition + .map(|d| d.respond_to_allowlist.as_slice()) + .unwrap_or(&[]), + ) + .map_err(|e| format!("definition respond-to allowlist is invalid: {e}"))? + } else { + input_allowlist + }; + (mode, list) + } + None => (RespondTo::default(), input_allowlist), + }, + }; + if respond_to == RespondTo::Allowlist && respond_to_allowlist.is_empty() { + return Err( + "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), + ); + } + + let non_blank = |v: Option| v.filter(|s| !s.trim().is_empty()); + let mcp_toolsets = non_blank(input_mcp_toolsets) + .or_else(|| non_blank(definition.and_then(|d| d.mcp_toolsets.clone()))); + + let parallelism = match input_parallelism { + // Explicit input is validated here too (not just at the command + // call sites) so the "validated when present" contract on + // `MintBehavioralDefaults.parallelism` is unskippable. + Some(count) if (1..=32).contains(&count) => Some(count), + Some(count) => { + return Err(format!( + "parallelism {count} is out of range (must be between 1 and 32)" + )) + } + None => match definition.and_then(|d| d.parallelism) { + Some(count) if (1..=32).contains(&count) => Some(count), + Some(count) => { + return Err(format!( + "parallelism {count} on the linked agent definition is out of range (must be between 1 and 32)" + )) + } + None => None, + }, + }; + + Ok(MintBehavioralDefaults { + respond_to, + respond_to_allowlist, + mcp_toolsets, + parallelism, + }) +} + +mod requests; +pub use requests::*; + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/types/requests.rs b/desktop/src-tauri/src/managed_agents/types/requests.rs new file mode 100644 index 0000000000..9838f55da8 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/types/requests.rs @@ -0,0 +1,314 @@ +//! Persona command request types, split from `types.rs` (file-size cap). + +use std::collections::BTreeMap; + +use serde::Deserialize; + +use super::{validate_respond_to_allowlist, PersonaRecord, RespondTo}; + +/// The NIP-AP behavioral quad as one grouped request field. +/// +/// Grouped (not flat) because `update_persona` has legacy callers that don't +/// send behavioral fields at all — flat replace semantics would silently wipe +/// a stored quad on every team-import edit. Absent group = don't touch the +/// stored quad; present group = validate and replace all four as a unit +/// (mode and allowlist must travel together). +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PersonaBehaviorRequest { + #[serde(default)] + pub respond_to: Option, + #[serde(default)] + pub respond_to_allowlist: Vec, + #[serde(default)] + pub mcp_toolsets: Option, + #[serde(default)] + pub parallelism: Option, +} + +/// Validate a behavior group and apply it onto a persona record. +/// +/// This is the single write path for definition behavioral fields — both +/// `create_persona` and `update_persona` route through it, so neither can +/// skip validation. `None` leaves the record's stored quad untouched (the +/// legacy-caller wipe hazard); `Some` normalizes the allowlist +/// (`validate_respond_to_allowlist`), rejects allowlist mode with an empty +/// list (the spawn-time crash-loop `build_respond_to_env` errors on), rejects +/// out-of-range parallelism, and stores the quad in wire shape. +pub fn apply_persona_behavior( + record: &mut PersonaRecord, + behavior: Option, +) -> Result<(), String> { + let Some(behavior) = behavior else { + return Ok(()); + }; + + let allowlist = validate_respond_to_allowlist(&behavior.respond_to_allowlist)?; + if behavior.respond_to == Some(RespondTo::Allowlist) && allowlist.is_empty() { + return Err( + "respond-to mode 'allowlist' requires at least one pubkey in the allowlist".to_string(), + ); + } + if let Some(count) = behavior.parallelism { + if !(1..=32).contains(&count) { + return Err(format!( + "parallelism {count} is out of range (must be between 1 and 32)" + )); + } + } + + record.respond_to = behavior.respond_to.map(|mode| mode.as_str().to_string()); + // The allowlist only means something in allowlist mode; storing it for + // other modes would republish stale pubkeys the author didn't choose. + record.respond_to_allowlist = if behavior.respond_to == Some(RespondTo::Allowlist) { + allowlist + } else { + Vec::new() + }; + record.mcp_toolsets = behavior + .mcp_toolsets + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()); + record.parallelism = behavior.parallelism; + Ok(()) +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CreatePersonaRequest { + pub display_name: String, + pub avatar_url: Option, + pub system_prompt: String, + #[serde(default)] + pub runtime: Option, + #[serde(default)] + pub model: Option, + #[serde(default)] + pub provider: Option, + #[serde(default)] + pub name_pool: Vec, + /// Environment variables for agents created from this persona. + #[serde(default)] + pub env_vars: BTreeMap, + /// NIP-AP behavioral quad. Absent = quad stays unset. + #[serde(default)] + pub behavior: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct UpdatePersonaRequest { + pub id: String, + pub display_name: String, + pub avatar_url: Option, + pub system_prompt: String, + #[serde(default)] + pub runtime: Option, + #[serde(default)] + pub model: Option, + #[serde(default)] + pub provider: Option, + #[serde(default)] + pub name_pool: Vec, + /// Environment variables for agents created from this persona. + /// + /// Absent (`None`) = don't touch the stored value (caller didn't include + /// the field). `Some(map)` = replace entirely (empty map clears all). + /// Defaulting an omitted field to an empty map would silently erase + /// stored credentials when an unrelated field is edited. + #[serde(default)] + pub env_vars: Option>, + /// NIP-AP behavioral quad. Same absent-vs-present contract as `env_vars`: + /// absent = don't touch the stored quad (legacy callers don't send it), + /// present = validate and replace all four fields as a unit. + #[serde(default)] + pub behavior: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + + fn record_with_quad() -> PersonaRecord { + let mut record = record_without_quad(); + record.respond_to = Some("allowlist".to_string()); + record.respond_to_allowlist = vec!["a".repeat(64)]; + record.mcp_toolsets = Some("developer".to_string()); + record.parallelism = Some(4); + record + } + + fn record_without_quad() -> PersonaRecord { + PersonaRecord { + id: "p-1".to_string(), + display_name: "Test".to_string(), + avatar_url: None, + system_prompt: "prompt".to_string(), + runtime: None, + model: None, + provider: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + source_team: None, + source_team_persona_slug: None, + env_vars: BTreeMap::new(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + created_at: "2026-01-01T00:00:00Z".to_string(), + updated_at: "2026-01-01T00:00:00Z".to_string(), + } + } + + /// The anchor regression row: an absent behavior group must leave a + /// stored quad untouched — legacy update_persona callers (team import, + /// profile panel) send no behavior field and must not wipe it. + #[test] + fn absent_behavior_leaves_stored_quad_untouched() { + let mut record = record_with_quad(); + apply_persona_behavior(&mut record, None).unwrap(); + assert_eq!(record.respond_to.as_deref(), Some("allowlist")); + assert_eq!(record.respond_to_allowlist, vec!["a".repeat(64)]); + assert_eq!(record.mcp_toolsets.as_deref(), Some("developer")); + assert_eq!(record.parallelism, Some(4)); + } + + #[test] + fn present_behavior_replaces_all_four_as_a_unit() { + let mut record = record_with_quad(); + apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + respond_to: Some(RespondTo::Anyone), + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, + }), + ) + .unwrap(); + assert_eq!(record.respond_to.as_deref(), Some("anyone")); + assert!(record.respond_to_allowlist.is_empty()); + assert_eq!(record.mcp_toolsets, None); + assert_eq!(record.parallelism, None); + } + + #[test] + fn allowlist_mode_with_empty_list_is_rejected() { + let mut record = record_without_quad(); + let err = apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + respond_to: Some(RespondTo::Allowlist), + respond_to_allowlist: Vec::new(), + ..Default::default() + }), + ) + .unwrap_err(); + assert!(err.contains("allowlist"), "{err}"); + // Rejection must not half-apply: the record stays untouched. + assert_eq!(record.respond_to, None); + } + + #[test] + fn allowlist_entries_are_normalized_via_the_shared_validator() { + let mut record = record_without_quad(); + let upper = "A".repeat(64); + apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + respond_to: Some(RespondTo::Allowlist), + respond_to_allowlist: vec![upper.clone(), upper], + ..Default::default() + }), + ) + .unwrap(); + // Lowercased and deduplicated, matching the instance-side chokepoint. + assert_eq!(record.respond_to_allowlist, vec!["a".repeat(64)]); + } + + #[test] + fn invalid_allowlist_entry_is_rejected() { + let mut record = record_without_quad(); + let err = apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + respond_to: Some(RespondTo::Allowlist), + respond_to_allowlist: vec!["not-hex".to_string()], + ..Default::default() + }), + ) + .unwrap_err(); + assert!(err.contains("64 hex"), "{err}"); + } + + #[test] + fn allowlist_is_dropped_for_non_allowlist_modes() { + let mut record = record_without_quad(); + apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + respond_to: Some(RespondTo::OwnerOnly), + respond_to_allowlist: vec!["b".repeat(64)], + ..Default::default() + }), + ) + .unwrap(); + assert!( + record.respond_to_allowlist.is_empty(), + "stale pubkeys must not be stored alongside a non-allowlist mode" + ); + } + + /// Pinky's loop row: an applied behavior group must flow through + /// `persona_event_content` so the republished 30175 carries the edited + /// quad — the write path and the publish path cannot drift apart. + #[test] + fn applied_behavior_flows_into_persona_event_content() { + let mut record = record_without_quad(); + apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + respond_to: Some(RespondTo::Allowlist), + respond_to_allowlist: vec!["c".repeat(64)], + mcp_toolsets: Some("developer".to_string()), + parallelism: Some(3), + }), + ) + .unwrap(); + let content = crate::managed_agents::persona_events::persona_event_content(&record); + assert_eq!(content.respond_to.as_deref(), Some("allowlist")); + assert_eq!(content.respond_to_allowlist, vec!["c".repeat(64)]); + assert_eq!(content.mcp_toolsets.as_deref(), Some("developer")); + assert_eq!(content.parallelism, Some(3)); + } + + #[test] + fn parallelism_out_of_range_is_rejected_and_blank_toolsets_normalize_to_none() { + let mut record = record_without_quad(); + for bad in [0u32, 33] { + let err = apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + parallelism: Some(bad), + ..Default::default() + }), + ) + .unwrap_err(); + assert!(err.contains("out of range"), "{err}"); + } + + apply_persona_behavior( + &mut record, + Some(PersonaBehaviorRequest { + mcp_toolsets: Some(" ".to_string()), + parallelism: Some(8), + ..Default::default() + }), + ) + .unwrap(); + assert_eq!(record.mcp_toolsets, None, "blank toolsets never persist"); + assert_eq!(record.parallelism, Some(8)); + } +} diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 752a7a04df..204f6d2f9a 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -485,6 +485,10 @@ fn sample_persona() -> PersonaRecord { source_team: Some("team-1".to_string()), source_team_persona_slug: Some("helper".to_string()), env_vars: [("K".to_string(), "v".to_string())].into_iter().collect(), + respond_to: None, + respond_to_allowlist: Vec::new(), + mcp_toolsets: None, + parallelism: None, created_at: "2026-01-01T00:00:00Z".to_string(), updated_at: "2026-01-02T00:00:00Z".to_string(), } @@ -534,3 +538,133 @@ fn empty_prompt_folds_to_none() { persona.system_prompt = String::new(); assert_eq!(persona.into_agent_record().system_prompt, None); } + +// ── Mint-time behavioral defaults (B5 quad activation) ────────────────────── + +use super::resolve_mint_behavioral_defaults; + +fn quad_definition(respond_to: &str, allowlist: Vec<&str>) -> PersonaRecord { + let mut persona = sample_persona(); + persona.respond_to = Some(respond_to.to_string()); + persona.respond_to_allowlist = allowlist.into_iter().map(str::to_string).collect(); + persona.mcp_toolsets = Some("default,canvas".to_string()); + persona.parallelism = Some(8); + persona +} + +#[test] +fn mint_explicit_input_wins_over_definition() { + let definition = quad_definition("anyone", vec![]); + let minted = resolve_mint_behavioral_defaults( + Some(RespondTo::OwnerOnly), + Vec::new(), + Some("default".to_string()), + Some(2), + Some(&definition), + ) + .unwrap(); + assert_eq!(minted.respond_to, RespondTo::OwnerOnly); + assert_eq!(minted.mcp_toolsets.as_deref(), Some("default")); + assert_eq!(minted.parallelism, Some(2)); +} + +#[test] +fn mint_copies_definition_quad_when_input_silent() { + let allow = "a".repeat(64); + let definition = quad_definition("allowlist", vec![&allow]); + let minted = + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition)).unwrap(); + assert_eq!(minted.respond_to, RespondTo::Allowlist); + assert_eq!(minted.respond_to_allowlist, vec![allow]); + assert_eq!(minted.mcp_toolsets.as_deref(), Some("default,canvas")); + assert_eq!(minted.parallelism, Some(8)); +} + +#[test] +fn mint_without_definition_or_input_uses_client_defaults() { + let minted = resolve_mint_behavioral_defaults(None, Vec::new(), None, None, None).unwrap(); + assert_eq!(minted.respond_to, RespondTo::default()); + assert!(minted.respond_to_allowlist.is_empty()); + assert_eq!(minted.mcp_toolsets, None); + assert_eq!(minted.parallelism, None); +} + +#[test] +fn mint_fails_loudly_on_unknown_definition_respond_to() { + // A typo'd mode must never silently become owner-only — the definition + // author intended SOMETHING, and guessing which thing is the one wrong + // move. The error must carry the offending string. + let definition = quad_definition("allowlst", vec![]); + let err = resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition)) + .unwrap_err(); + assert!( + err.contains("allowlst"), + "error must name the bad mode: {err}" + ); +} + +#[test] +fn mint_fails_loudly_on_empty_definition_allowlist() { + // Inbound definitions bypass the dialog guard entirely — the mint + // boundary is the backstop against a crash-looping instance. + let definition = quad_definition("allowlist", vec![]); + let err = resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition)) + .unwrap_err(); + assert!( + err.contains("at least one pubkey"), + "unexpected error: {err}" + ); +} + +#[test] +fn mint_fails_loudly_on_out_of_range_definition_parallelism() { + let mut definition = quad_definition("anyone", vec![]); + definition.parallelism = Some(64); + let err = resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition)) + .unwrap_err(); + assert!(err.contains("64"), "error must name the bad value: {err}"); +} + +#[test] +fn mint_normalizes_definition_allowlist_from_wire() { + let upper = "A".repeat(64); + let definition = quad_definition("allowlist", vec![&upper]); + let minted = + resolve_mint_behavioral_defaults(None, Vec::new(), None, None, Some(&definition)).unwrap(); + assert_eq!(minted.respond_to_allowlist, vec!["a".repeat(64)]); +} + +#[test] +fn mint_resolves_each_quad_field_independently() { + // PR #1667 review (convergent): the input-wins rule is per-FIELD, not + // all-or-nothing — explicit toolsets must not stop respond_to or + // parallelism from inheriting. + let definition = quad_definition("anyone", vec![]); + let minted = resolve_mint_behavioral_defaults( + None, + Vec::new(), + Some("explicit-toolsets".to_string()), + None, + Some(&definition), + ) + .unwrap(); + assert_eq!(minted.respond_to, RespondTo::Anyone, "inherited"); + assert_eq!( + minted.mcp_toolsets.as_deref(), + Some("explicit-toolsets"), + "explicit input wins" + ); + assert_eq!(minted.parallelism, Some(8), "inherited"); +} + +#[test] +fn mint_rejects_out_of_range_input_parallelism() { + // The "validated when present" contract on MintBehavioralDefaults holds + // for the INPUT branch too, not just definition values. + let err = resolve_mint_behavioral_defaults(None, Vec::new(), None, Some(64), None).unwrap_err(); + assert!(err.contains("64"), "error must name the bad value: {err}"); + assert!( + !err.contains("definition"), + "input-branch error must not blame the definition: {err}" + ); +} diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 816b6b072f..efbc1e1fb8 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -156,6 +156,11 @@ pub fn run_boot_migrations(app: &tauri::AppHandle) { // Post-fold readers of the runtime map (`load_persona_runtimes`) fall // back to the unified store's definitions. fold_personas_into_agent_store(app); + // B5: manufacture definitions for standalone agents AFTER the fold (so + // pre-existing definition slugs are present for collision checks) and + // before event sync republishes — the backfilled link is what flips the + // 30177 projection to its slim shape. + backfill_standalone_agents(app); if let Err(e) = crate::managed_agents::sync_team_personas(app) { eprintln!("buzz-desktop: sync-team-personas: {e}"); } @@ -1259,6 +1264,8 @@ pub use materialize::materialize_agent_runtimes; mod fold; pub use fold::fold_personas_into_agent_store; use fold::load_persona_runtimes; +mod backfill; +pub use backfill::backfill_standalone_agents; #[cfg(test)] #[path = "migration_test_support.rs"] diff --git a/desktop/src-tauri/src/migration/backfill.rs b/desktop/src-tauri/src/migration/backfill.rs new file mode 100644 index 0000000000..484e464bc0 --- /dev/null +++ b/desktop/src-tauri/src/migration/backfill.rs @@ -0,0 +1,140 @@ +//! B5 (unified agent model): one-time backfill of standalone agents into +//! definition-linked records. Every keyed record with `persona_id: None` +//! gets a key-less definition manufactured from its own settings, retiring +//! the standalone-create pattern (Wes's Option 2: backfill, not grandfather). + +use std::path::Path; + +use crate::managed_agents::{ + persona_events::{persona_content_hash, persona_event_content}, + ManagedAgentRecord, +}; + +/// Manufacture definitions for standalone agents (B5 backfill). +/// +/// For each keyed record with `persona_id: None`, append a key-less +/// definition record snapshotting the agent's own config and link the agent +/// to it. Safety rails (pinned in the B5 review gates): +/// - **Idempotent**: linked records are skipped, so a second run is a no-op. +/// - **`.bak` create-if-absent**: the pre-migration backup is taken once and +/// never clobbered — a partial first run must not replace the pristine +/// backup with a half-migrated snapshot on re-run. +/// - **Fail loudly per record**: a record that cannot be backfilled (slug +/// collision) is logged and skipped; the rest proceed. +/// - **No behavior change**: the definition snapshots the record's own +/// values (prompt present-even-if-empty via `to_persona_view`'s +/// `unwrap_or_default`, env COPIED so later instances inherit a working +/// config, quad copied to the definition defaults) and the record gains +/// `persona_source_version` = the new definition's content hash, so +/// neither `spawn_config_hash` nor the drift badge moves. +/// +/// The manufactured definition's slug is the agent's pubkey: 64-hex passes +/// the NIP-AP slug grammar on both relay and desktop ends, and agent pubkeys +/// are unique, so the coordinate is collision-free by construction. +pub fn backfill_standalone_agents(app: &tauri::AppHandle) { + let Ok(base_dir) = crate::managed_agents::managed_agents_base_dir(app) else { + return; + }; + match backfill_standalone_agents_in_dir(&base_dir) { + Ok(0) => {} + Ok(backfilled) => { + eprintln!( + "buzz-desktop: standalone-backfill: {backfilled} agents linked to manufactured definitions" + ); + } + Err(e) => eprintln!("buzz-desktop: standalone-backfill: {e}"), + } +} + +/// Core backfill logic, decoupled from the Tauri `AppHandle` for testing. +/// Returns the number of records backfilled (0 = nothing to do). +fn backfill_standalone_agents_in_dir(base_dir: &Path) -> Result { + let agents_path = base_dir.join("managed-agents.json"); + if !agents_path.exists() { + return Ok(0); + } + let content = std::fs::read_to_string(&agents_path) + .map_err(|e| format!("failed to read managed-agents.json: {e}"))?; + let mut all: Vec = serde_json::from_str(&content) + .map_err(|e| format!("failed to parse managed-agents.json: {e}"))?; + + let needs_backfill = + |record: &ManagedAgentRecord| !record.pubkey.is_empty() && record.persona_id.is_none(); + if !all.iter().any(needs_backfill) { + return Ok(0); + } + + // Pre-migration backup, taken ONCE: a re-run after a partial failure must + // not overwrite the pristine backup with a half-migrated snapshot. + let bak_path = base_dir.join("managed-agents.json.pre-backfill.bak"); + if !bak_path.exists() { + std::fs::write(&bak_path, &content) + .map_err(|e| format!("failed to write pre-backfill backup: {e}"))?; + } + + let existing_slugs: std::collections::HashSet = + all.iter().filter_map(|r| r.slug.clone()).collect(); + + let mut manufactured: Vec = Vec::new(); + let mut backfilled = 0usize; + for record in all.iter_mut().filter(|r| needs_backfill(r)) { + // Pubkeys are unique so this cannot fire against another manufactured + // definition — only against a pre-existing definition improbably + // slugged as this agent's pubkey. Fail loudly, skip, continue: the + // agent keeps working persona-less (`persona_id: None`), and the + // backfill retries it on every boot. Recovery path: delete or re-slug + // the colliding definition, then relaunch. + if existing_slugs.contains(&record.pubkey) { + eprintln!( + "buzz-desktop: standalone-backfill: slug collision for agent {} — skipped; \ + delete or re-slug the colliding definition to let the next launch backfill it", + record.pubkey + ); + continue; + } + + // Snapshot the record's own config as a definition. Via the same + // fold path every definition takes: a temporary persona view of the + // record (prompt unwrap_or_default = present-even-if-empty — the + // heal source old devices hard-require) folded into a key-less + // definition record. Quad + env come along so future instances + // minted from this definition inherit a working config. + let mut view_source = record.clone(); + view_source.slug = Some(record.pubkey.clone()); + // Standalone agents have no definition-level quad — the INSTANCE + // fields are the author's intent; copy them up. + view_source.definition_respond_to = Some(record.respond_to.as_str().to_string()); + view_source.definition_respond_to_allowlist = record.respond_to_allowlist.clone(); + view_source.definition_mcp_toolsets = record.mcp_toolsets.clone(); + view_source.definition_parallelism = Some(record.parallelism); + let Some(persona_view) = view_source.to_persona_view() else { + eprintln!( + "buzz-desktop: standalone-backfill: agent {} produced no persona view — skipped", + record.pubkey + ); + continue; + }; + + // Link the record BEFORE computing the version so the hash covers the + // definition exactly as manufactured. + let source_version = persona_content_hash(&persona_event_content(&persona_view)); + let definition = persona_view.into_agent_record(); + record.persona_id = Some(record.pubkey.clone()); + record.persona_source_version = Some(source_version); + manufactured.push(definition); + backfilled += 1; + } + + if backfilled == 0 { + return Ok(0); + } + all.extend(manufactured); + let payload = serde_json::to_vec_pretty(&all) + .map_err(|e| format!("failed to serialize unified store: {e}"))?; + crate::managed_agents::atomic_write_json_restricted(&agents_path, &payload)?; + Ok(backfilled) +} + +#[cfg(test)] +#[path = "backfill_tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/migration/backfill_tests.rs b/desktop/src-tauri/src/migration/backfill_tests.rs new file mode 100644 index 0000000000..b3798c2309 --- /dev/null +++ b/desktop/src-tauri/src/migration/backfill_tests.rs @@ -0,0 +1,277 @@ +use super::backfill_standalone_agents_in_dir; +use crate::managed_agents::spawn_hash::spawn_config_hash; +use crate::managed_agents::{ManagedAgentRecord, PersonaRecord}; +use crate::migration::test_support::{read_agents_json, write_agents_json}; +use std::path::Path; + +fn standalone_agent_json(name: &str, pubkey: &str, prompt: Option<&str>) -> serde_json::Value { + serde_json::json!({ + "name": name, + "pubkey": pubkey, + "relay_url": "ws://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 4, + "system_prompt": prompt, + "model": "gpt-x", + "provider": "openai", + "respond_to": "anyone", + "env_vars": { "API_KEY": "secret" }, + "start_on_app_launch": true, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }) +} + +fn load_typed(dir: &Path) -> Vec { + let content = std::fs::read_to_string(dir.join("agents").join("managed-agents.json")).unwrap(); + serde_json::from_str(&content).unwrap() +} + +fn base(dir: &Path) -> std::path::PathBuf { + dir.join("agents") +} + +#[test] +fn backfill_links_standalone_agent_to_manufactured_definition() { + let dir = tempfile::tempdir().unwrap(); + let pubkey = "a".repeat(64); + write_agents_json( + dir.path(), + &serde_json::json!([standalone_agent_json( + "Solo", + &pubkey, + Some("You are Solo.") + )]), + ); + + let backfilled = backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); + assert_eq!(backfilled, 1); + + let records = load_typed(dir.path()); + assert_eq!(records.len(), 2, "instance + manufactured definition"); + + let instance = records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); + assert_eq!(instance.persona_id.as_deref(), Some(pubkey.as_str())); + assert!(instance.persona_source_version.is_some()); + + let definition = records.iter().find(|r| r.pubkey.is_empty()).unwrap(); + assert_eq!(definition.slug.as_deref(), Some(pubkey.as_str())); + assert_eq!(definition.system_prompt.as_deref(), Some("You are Solo.")); + assert_eq!(definition.model.as_deref(), Some("gpt-x")); + // Env COPIED (B5 pin): later instances inherit a working config. + assert_eq!( + definition.env_vars.get("API_KEY").map(String::as_str), + Some("secret") + ); + // Instance quad copied up as the definition's defaults. + assert_eq!(definition.definition_respond_to.as_deref(), Some("anyone")); + assert_eq!(definition.definition_parallelism, Some(4)); + + // The recorded version matches the definition's actual content hash — + // the drift badge starts clean. + let view = definition.to_persona_view().unwrap(); + let expected = crate::managed_agents::persona_events::persona_content_hash( + &crate::managed_agents::persona_events::persona_event_content(&view), + ); + assert_eq!( + instance.persona_source_version.as_deref(), + Some(expected.as_str()) + ); +} + +#[test] +fn backfilled_definition_carries_prompt_present_even_if_empty() { + // LOAD-BEARING (B5 gates): old readers hard-fail on an absent prompt. A + // prompt-less backfilled definition would leave a wiped old device with + // no heal source, permanently. `PersonaRecord.system_prompt` is a plain + // String and the outbound projection wraps it in `Some` unconditionally + // — this row pins that chain against refactors. + let dir = tempfile::tempdir().unwrap(); + let pubkey = "b".repeat(64); + write_agents_json( + dir.path(), + &serde_json::json!([standalone_agent_json("NoPrompt", &pubkey, None)]), + ); + + backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); + + let records = load_typed(dir.path()); + let definition = records.iter().find(|r| r.pubkey.is_empty()).unwrap(); + let view: PersonaRecord = definition.to_persona_view().unwrap(); + assert_eq!(view.system_prompt, "", "empty, not absent"); + let content = crate::managed_agents::persona_events::persona_event_content(&view); + assert_eq!( + content.system_prompt.as_deref(), + Some(""), + "wire projection must carry Some(\"\") — the old-reader heal source" + ); +} + +#[test] +fn backfill_of_promptless_record_keeps_spawn_hash_stable() { + // B5 hash row 2: pre-backfill the record hashes prompt None; post-backfill + // the prospective re-snapshot pulls Some("") from the manufactured + // definition. The spawn layer treats an empty prompt as no prompt (env + // absent either way), so the hash must not move — otherwise every + // prompt-less standalone agent lights the restart badge on upgrade. + let dir = tempfile::tempdir().unwrap(); + let pubkey = "c".repeat(64); + write_agents_json( + dir.path(), + &serde_json::json!([standalone_agent_json("NoPrompt", &pubkey, None)]), + ); + + let pre_records = load_typed(dir.path()); + let pre_instance = pre_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); + let hash_before = spawn_config_hash(pre_instance, &[], "wss://ws.example"); + + backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); + + let post_records = load_typed(dir.path()); + let post_instance = post_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); + let personas: Vec = post_records + .iter() + .filter_map(|r| r.to_persona_view()) + .collect(); + let hash_after = spawn_config_hash(post_instance, &personas, "wss://ws.example"); + + assert_eq!( + hash_before, hash_after, + "backfill must not flip the restart badge for prompt-less agents" + ); +} + +#[test] +fn backfill_of_prompted_record_keeps_spawn_hash_stable() { + // The general no-behavior-change rail: a standalone agent WITH config + // must also hash identically across backfill (the definition snapshots + // the record's own values, so the re-snapshot writes back what is + // already there). + let dir = tempfile::tempdir().unwrap(); + let pubkey = "d".repeat(64); + write_agents_json( + dir.path(), + &serde_json::json!([standalone_agent_json( + "Solo", + &pubkey, + Some("You are Solo.") + )]), + ); + + let pre_records = load_typed(dir.path()); + let pre_instance = pre_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); + let hash_before = spawn_config_hash(pre_instance, &[], "wss://ws.example"); + + backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(); + + let post_records = load_typed(dir.path()); + let post_instance = post_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); + let personas: Vec = post_records + .iter() + .filter_map(|r| r.to_persona_view()) + .collect(); + let hash_after = spawn_config_hash(post_instance, &personas, "wss://ws.example"); + + assert_eq!(hash_before, hash_after); +} + +#[test] +fn second_run_is_a_no_op_and_preserves_pristine_backup() { + // B5 gates: double-run idempotence + create-if-absent .bak. Run 1 + // migrates; run 2 must change nothing and must NOT clobber the pristine + // pre-migration backup with a half-migrated snapshot. + let dir = tempfile::tempdir().unwrap(); + let pubkey = "e".repeat(64); + write_agents_json( + dir.path(), + &serde_json::json!([standalone_agent_json("Solo", &pubkey, Some("P"))]), + ); + let pristine = std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(); + + assert_eq!( + backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(), + 1 + ); + let after_first = + std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(); + + assert_eq!( + backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(), + 0, + "second run is a no-op" + ); + let after_second = + std::fs::read_to_string(base(dir.path()).join("managed-agents.json")).unwrap(); + assert_eq!(after_first, after_second, "store untouched by re-run"); + + let bak = + std::fs::read_to_string(base(dir.path()).join("managed-agents.json.pre-backfill.bak")) + .unwrap(); + assert_eq!( + bak, pristine, + "backup is the PRE-migration state, never clobbered" + ); +} + +#[test] +fn definitions_and_linked_records_are_untouched() { + // Already-linked instances and existing definitions pass through + // byte-identical; a store with nothing to backfill takes no backup. + let dir = tempfile::tempdir().unwrap(); + let pubkey = "f".repeat(64); + let mut linked = standalone_agent_json("Linked", &pubkey, Some("P")); + linked["persona_id"] = serde_json::json!("some-definition"); + write_agents_json(dir.path(), &serde_json::json!([linked])); + + assert_eq!( + backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(), + 0 + ); + assert!( + !base(dir.path()) + .join("managed-agents.json.pre-backfill.bak") + .exists(), + "no work, no backup" + ); + let records = read_agents_json(dir.path()); + assert_eq!(records.len(), 1, "nothing manufactured"); +} + +#[test] +fn slug_collision_fails_loudly_per_record_and_continues() { + // A pre-existing definition improbably slugged as an agent's pubkey: + // that record is skipped (logged), the rest proceed. + let dir = tempfile::tempdir().unwrap(); + let colliding = "1".repeat(64); + let clean = "2".repeat(64); + let mut definition = standalone_agent_json("Def", "", Some("P")); + definition["slug"] = serde_json::json!(colliding.clone()); + definition["pubkey"] = serde_json::json!(""); + write_agents_json( + dir.path(), + &serde_json::json!([ + definition, + standalone_agent_json("Collides", &colliding, Some("P")), + standalone_agent_json("Clean", &clean, Some("P")), + ]), + ); + + assert_eq!( + backfill_standalone_agents_in_dir(&base(dir.path())).unwrap(), + 1, + "collision skipped, clean record backfilled" + ); + let records = load_typed(dir.path()); + let collided = records.iter().find(|r| r.pubkey == colliding).unwrap(); + assert_eq!(collided.persona_id, None, "collided record left standalone"); + let clean_rec = records.iter().find(|r| r.pubkey == clean).unwrap(); + assert_eq!(clean_rec.persona_id.as_deref(), Some(clean.as_str())); +} diff --git a/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs b/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs index 01cb04fe54..dfd287ad23 100644 --- a/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs +++ b/desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { availableRuntimesForStart, buildInstanceInputForDefinition, + mintDefinitionWithPreflight, resolveStartRuntimeForDefinition, } from "./instanceInputForDefinition.ts"; @@ -127,6 +128,157 @@ test("mapping carries the runtime and definition fields", async () => { assert.deepEqual(input.backend, { type: "local" }); }); +test("no backend intent is byte-identical to the pre-intent mapping", async () => { + // The 3 pre-B5 call sites (useManagedAgentActions, usePersonaActions, + // UserProfilePanel) pass no intent; their output must not move. + const input = await buildInstanceInputForDefinition(persona(), gooseRuntime); + assert.deepEqual(input, { + name: "Test Agent", + personaId: "p-1", + systemPrompt: "prompt", + avatarUrl: "https://example.com/a.png", + acpCommand: "buzz-acp", + agentCommand: "goose-cmd", + agentArgs: ["--acp"], + mcpCommand: "goose-mcp", + harnessOverride: true, + model: undefined, + spawnAfterCreate: true, + startOnAppLaunch: true, + backend: { type: "local" }, + }); +}); + +test("provider intent forces startOnAppLaunch off and omits local commands", async () => { + const input = await buildInstanceInputForDefinition( + persona(), + gooseRuntime, + undefined, + { type: "provider", id: "blox", config: { region: "us" } }, + ); + assert.deepEqual(input.backend, { + type: "provider", + id: "blox", + config: { region: "us" }, + }); + assert.equal(input.startOnAppLaunch, false, "remote agents never auto-start"); + assert.equal(input.spawnAfterCreate, true); + assert.equal(input.harnessOverride, false); + // Provider agents spawn no local ACP — the legacy provider branch omitted + // all local commands and model/provider, and so does the intent path. + for (const key of [ + "acpCommand", + "agentCommand", + "agentArgs", + "mcpCommand", + "model", + "envVars", + "relayMesh", + ]) { + assert.equal(key in input, false, `provider intent must omit ${key}`); + } + assert.equal(input.personaId, "p-1", "definition link is kept"); + assert.equal(input.systemPrompt, "prompt"); +}); + +test("mesh intent applies the preset patch as instance-override state", async () => { + const patch = { + acpCommand: "buzz-acp", + agentCommand: "buzz-agent", + agentArgs: ["acp"], + mcpCommand: "", + model: "mesh/model:Q4", + envVars: { OPENAI_BASE_URL: "http://127.0.0.1:9337/v1" }, + }; + const input = await buildInstanceInputForDefinition( + persona(), + gooseRuntime, + undefined, + { + type: "mesh", + modelId: "mesh/model:Q4", + target: { endpointAddr: "10.0.0.1:9337", modelId: "mesh/model:Q4" }, + patch, + }, + ); + assert.equal(input.agentCommand, "buzz-agent"); + assert.deepEqual(input.agentArgs, ["acp"]); + assert.equal(input.model, "mesh/model:Q4"); + assert.deepEqual(input.envVars, patch.envVars); + assert.deepEqual(input.relayMesh, { modelRef: "mesh/model:Q4" }); + assert.equal( + input.harnessOverride, + true, + "preset commands deliberately override the definition runtime", + ); + assert.equal( + input.startOnAppLaunch, + false, + "mesh agents need a fresh serve target; never auto-restore", + ); + assert.deepEqual(input.backend, { type: "local" }); + assert.equal(input.personaId, "p-1", "definition link is kept"); + // The patch must be copied, not aliased — a caller mutating its patch + // after the fact must not reach into the built input. + patch.agentArgs.push("mutated"); + patch.envVars.INJECTED = "x"; + assert.deepEqual(input.agentArgs, ["acp"]); + assert.equal("INJECTED" in input.envVars, false); +}); + +test("preflight runs only for mesh intent, before the mint, and a failure never mints", async () => { + const calls = []; + const prepare = async (modelId, target) => { + calls.push(["prepare", modelId, target]); + }; + const mint = async () => { + calls.push(["mint"]); + return "definition"; + }; + + // Local (no intent) and provider intents mint immediately, no preflight. + assert.equal( + await mintDefinitionWithPreflight(undefined, prepare, mint), + "definition", + ); + await mintDefinitionWithPreflight(null, prepare, mint); + await mintDefinitionWithPreflight( + { type: "provider", id: "blox", config: {} }, + prepare, + mint, + ); + assert.deepEqual( + calls, + [["mint"], ["mint"], ["mint"]], + "non-mesh intents must not preflight", + ); + + // Mesh intent preflights with the selected target BEFORE the mint. + calls.length = 0; + const target = { endpointAddr: "10.0.0.1:9337", modelId: "m" }; + await mintDefinitionWithPreflight( + { type: "mesh", modelId: "m", target, patch: {} }, + prepare, + mint, + ); + assert.deepEqual(calls, [["prepare", "m", target], ["mint"]]); + + // A preflight rejection propagates and the mint NEVER runs — a dead mesh + // target must not orphan a definition the user didn't ask for. + calls.length = 0; + await assert.rejects( + mintDefinitionWithPreflight( + { type: "mesh", modelId: "m", target, patch: {} }, + async () => { + throw new Error("target unreachable"); + }, + mint, + ), + /target unreachable/, + ); + assert.deepEqual(calls, [], "a failed preflight must not mint anything"); +}); + test("row 1: refuses when the configured runtime is not available", () => { assert.throws( () => diff --git a/desktop/src/features/agents/lib/instanceInputForDefinition.ts b/desktop/src/features/agents/lib/instanceInputForDefinition.ts index f4f20284fd..3faa5318ff 100644 --- a/desktop/src/features/agents/lib/instanceInputForDefinition.ts +++ b/desktop/src/features/agents/lib/instanceInputForDefinition.ts @@ -4,6 +4,8 @@ import type { AgentPersona, CreateManagedAgentInput, } from "@/shared/api/types"; +import type { MeshAgentPresetPatch } from "@/features/mesh-compute/applyMeshAgentPreset"; +import type { MeshServeTarget } from "@/shared/api/tauriMesh"; import { resolvePersonaRuntime, type ResolvePersonaRuntimeResult, @@ -61,6 +63,56 @@ export function resolveStartRuntimeForDefinition( return { runtime, warnings }; } +/** + * Where the started instance should run when the user picked something other + * than plain local in the definition-create flow (B5). Absent intent = + * today's local mapping, byte-identical. + * + * - `provider`: remote backend. Mirrors the legacy provider-mode create: + * no local ACP/agent/MCP commands are spawned, so none are set; + * `startOnAppLaunch` is forced false (remote agents don't auto-start with + * the desktop) and `spawnAfterCreate` true. + * - `mesh`: relay-mesh compute. The preset patch carries the instance + * commands/env the legacy dialog fanned into its field state; env lands in + * record env_vars (the instance-override layer — the dial pointer is + * per-instance runtime state, never definition env). `harnessOverride` + * is true because the preset commands deliberately override the + * definition's runtime preference. + */ +export type BackendIntent = + | { + type: "provider"; + id: string; + config: Record; + } + | { + type: "mesh"; + modelId: string; + /** The selected serve target — consumed by the pre-mint preflight. */ + target: MeshServeTarget; + patch: MeshAgentPresetPatch; + }; + +/** + * Run the relay-mesh client preflight for a mesh intent, then mint the + * definition. The preflight runs BEFORE the mint — definition included — so + * a dead mesh target aborts the whole create instead of orphaning a + * definition the user didn't ask for; a rejection propagates and `mint` is + * never invoked. Non-mesh intents mint immediately: the preflight never runs + * for local or provider creates. Preflight and mint live in one function so + * a caller cannot reorder them. + */ +export async function mintDefinitionWithPreflight( + backendIntent: BackendIntent | null | undefined, + prepare: (modelId: string, target: MeshServeTarget) => Promise, + mint: () => Promise, +): Promise { + if (backendIntent?.type === "mesh") { + await prepare(backendIntent.modelId, backendIntent.target); + } + return mint(); +} + /** * The single definition→instance mapping (Phase 1B.3.5 rows 2–4). Every * surface that creates a running instance from a definition builds its @@ -76,12 +128,15 @@ export function resolveStartRuntimeForDefinition( * - envVars are never seeded from the definition: record.env_vars is * agent overrides only and spawn merges the live definition env * underneath. Seeding would manufacture pseudo-overrides that mask - * later definition edits made before the first spawn. + * later definition edits made before the first spawn. (Mesh preset env is + * the deliberate exception: it is instance-override state, not + * definition env.) */ export async function buildInstanceInputForDefinition( persona: AgentPersona, runtime: AcpRuntime, upload?: UploadMediaBytes, + backendIntent?: BackendIntent, ): Promise { const avatarUrl = await resolveManagedAgentAvatarUrl( persona.avatarUrl, @@ -89,16 +144,54 @@ export async function buildInstanceInputForDefinition( runtime.avatarUrl, ); - return { + const base = { name: persona.displayName, + personaId: persona.id, + systemPrompt: persona.systemPrompt, + avatarUrl, + }; + + if (backendIntent?.type === "provider") { + return { + ...base, + harnessOverride: false, + spawnAfterCreate: true, + startOnAppLaunch: false, + backend: { + type: "provider", + id: backendIntent.id, + config: backendIntent.config, + }, + }; + } + + if (backendIntent?.type === "mesh") { + const { patch } = backendIntent; + return { + ...base, + acpCommand: patch.acpCommand, + agentCommand: patch.agentCommand, + agentArgs: [...patch.agentArgs], + mcpCommand: patch.mcpCommand, + harnessOverride: true, + model: patch.model, + envVars: { ...patch.envVars }, + relayMesh: { modelRef: backendIntent.modelId }, + spawnAfterCreate: true, + // Relay-mesh agents need a freshly selected serve target to start; + // do not auto-restore them later with only the saved model/env. + startOnAppLaunch: false, + backend: { type: "local" }, + }; + } + + return { + ...base, acpCommand: "buzz-acp", agentCommand: runtime.command, agentArgs: runtime.defaultArgs, mcpCommand: runtime.mcpCommand ?? "", - personaId: persona.id, harnessOverride: !persona.runtime || persona.runtime === runtime.id, - systemPrompt: persona.systemPrompt, - avatarUrl, model: persona.model ?? undefined, spawnAfterCreate: true, startOnAppLaunch: true, diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index c5d4c28829..74938b61a1 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -37,6 +37,12 @@ import { hasAdvancedEnvVars, hasText, } from "./personaDialogEnvVars"; +import { + behaviorForSubmit, + draftFromBehavior, + emptyPersonaBehaviorDraft, + personaBehaviorDraftValid, +} from "./personaBehaviorDraft"; import { AUTO_MODEL_DROPDOWN_VALUE, AUTO_PROVIDER_DROPDOWN_VALUE, @@ -107,6 +113,10 @@ type AgentDefinitionDialogProps = { * import button owns that slot (`canImportPersonaUpdate`). */ createFooterSlot?: React.ReactNode; + /** Rendered below the form fields in create mode only ("Where to run"). */ + createRunSection?: React.ReactNode; + /** Extra create-mode submit gate (e.g. incomplete provider config). */ + createSubmitBlocked?: boolean; }; const ADVANCED_FIELDS_MOTION_TRANSITION = { @@ -129,6 +139,8 @@ export function AgentDefinitionDialog({ onSubmit, onImportUpdateFile, createFooterSlot, + createRunSection, + createSubmitBlocked = false, }: AgentDefinitionDialogProps) { const [displayName, setDisplayName] = React.useState(""); const [avatarUrl, setAvatarUrl] = React.useState(""); @@ -141,6 +153,12 @@ export function AgentDefinitionDialog({ React.useState(false); const [namePoolText, setNamePoolText] = React.useState(""); const [envVars, setEnvVars] = React.useState({}); + const [behaviorDraft, setBehaviorDraft] = React.useState( + emptyPersonaBehaviorDraft, + ); + // The seed the draft is diffed against at submit: an untouched quad + // submits no behavior group, keeping unrelated edits hash-quiet. + const behaviorSeedRef = React.useRef(emptyPersonaBehaviorDraft); const [showAdvancedFields, setShowAdvancedFields] = React.useState(false); const [isAvatarUploadPending, setIsAvatarUploadPending] = React.useState(false); @@ -188,11 +206,17 @@ export function AgentDefinitionDialog({ const managedApiKeyEnvVar = getProviderApiKeyEnvVar( initialValues.provider ?? "", ); + const nextBehaviorDraft = draftFromBehavior(initialValues.behavior); + behaviorSeedRef.current = draftFromBehavior(initialValues.behavior); + setBehaviorDraft(nextBehaviorDraft); setNamePoolText(nextNamePoolText); setEnvVars(nextEnvVars); setShowAdvancedFields( nextNamePoolText.trim().length > 0 || - hasAdvancedEnvVars(nextEnvVars, managedApiKeyEnvVar), + hasAdvancedEnvVars(nextEnvVars, managedApiKeyEnvVar) || + nextBehaviorDraft.respondTo !== null || + nextBehaviorDraft.mcpToolsets.trim().length > 0 || + nextBehaviorDraft.parallelism.trim().length > 0, ); setIsAvatarUploadPending(false); setImportErrorMessage(null); @@ -276,6 +300,8 @@ export function AgentDefinitionDialog({ setIsCustomProviderEditing(false); setNamePoolText(""); setEnvVars({}); + setBehaviorDraft(emptyPersonaBehaviorDraft); + behaviorSeedRef.current = emptyPersonaBehaviorDraft; setShowAdvancedFields(false); setIsAvatarUploadPending(false); setImportErrorMessage(null); @@ -332,6 +358,11 @@ export function AgentDefinitionDialog({ : undefined, namePool: namePoolInput, envVars, + behavior: behaviorForSubmit( + behaviorDraft, + behaviorSeedRef.current, + "id" in initialValues, + ), }; if ("id" in initialValues) { @@ -435,6 +466,10 @@ export function AgentDefinitionDialog({ canSubmitPersonaDialog({ displayName, isPending }) && (!isCreateMode || runtime.trim().length > 0) && (!isCreateMode || selectedRuntimeIsAvailable) && + (!isCreateMode || !createSubmitBlocked) && + // Crash-loop guard, create AND edit: an empty allowlist would crash + // every instance minted from this definition at startup. + personaBehaviorDraftValid(behaviorDraft) && (!isExplicitModelRequired || model.trim().length > 0) && !isAvatarUploadPending; const { @@ -893,6 +928,8 @@ export function AgentDefinitionDialog({ ) : null} + {isCreateMode ? createRunSection : null} +
- {createDialogMode ? ( + {isCreateDialogOpen ? ( { - agents.setLogAgentPubkey(result.agent.pubkey); - agents.setCreatedAgent(result); - }} + mode="definition" onOpenChange={(open) => { if (!open) { - setCreateDialogMode(null); + setIsCreateDialogOpen(false); } }} onSubmitDefinition={personas.handleSubmit} diff --git a/desktop/src/features/agents/ui/CreateAgentDialog.tsx b/desktop/src/features/agents/ui/CreateAgentDialog.tsx deleted file mode 100644 index 2a638ef78d..0000000000 --- a/desktop/src/features/agents/ui/CreateAgentDialog.tsx +++ /dev/null @@ -1,877 +0,0 @@ -import { AlertTriangle, ChevronDown } from "lucide-react"; -import * as React from "react"; - -import { - useAcpRuntimesQuery, - useAvailableAcpRuntimes, - useBackendProvidersQuery, - useBakedBuildEnvKeysQuery, - useCreateManagedAgentMutation, - useManagedAgentPrereqsQuery, - useRuntimeFileConfigQuery, -} from "@/features/agents/hooks"; -import { probeBackendProvider } from "@/shared/api/tauri"; -import type { - BackendProviderProbeResult, - CreateManagedAgentInput, - CreateManagedAgentResponse, - RespondToMode, -} from "@/shared/api/types"; -import { Button } from "@/shared/ui/button"; -import { - Dialog, - DialogContent, - DialogDescription, - DialogHeader, - DialogTitle, -} from "@/shared/ui/dialog"; -import { - CreateAgentBasicsFields, - CreateAgentOptionToggles, - CreateAgentRuntimeField, - CreateAgentRuntimeFields, -} from "./CreateAgentDialogSections"; -import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor"; -import { - coerceConfigValues, - ProviderConfigFields, -} from "./ProviderConfigFields"; -import { CreateAgentRespondToField } from "./RespondToField"; -import { RelayMeshAgentSection } from "@/features/mesh-compute/ui/RelayMeshAgentSection"; -import { meshPrepareRelayMeshClient } from "@/shared/api/tauriMesh"; -import type { MeshServeTarget } from "@/shared/api/tauriMesh"; -import { useLastRuntime } from "@/features/agents/lib/useLastRuntime"; -import { - getBakedSatisfiedEnvKeys, - requiredCredentialEnvKeys, - runtimeSupportsLlmProviderSelection, - shouldClearKnownModelForSelectionScope, -} from "./personaDialogPickers"; -import { - selectionOnProviderDropdownChange, - selectionOnRuntimeChange, - type RuntimeModelProviderSelection, -} from "./runtimeModelProviderSelection"; -import { - AgentModelField, - AgentProviderField, -} from "./personaProviderModelFields"; -import { usePersonaModelDiscovery } from "./usePersonaModelDiscovery"; - -export function CreateAgentDialog({ - open, - onCreated, - onOpenChange, -}: { - open: boolean; - onCreated: (result: CreateManagedAgentResponse) => void; - onOpenChange: (open: boolean) => void; -}) { - const createMutation = useCreateManagedAgentMutation(); - const providersQuery = useAvailableAcpRuntimes({ enabled: open }); - const allProvidersQuery = useAcpRuntimesQuery({ enabled: open }); - const backendProvidersQuery = useBackendProvidersQuery({ enabled: open }); - const { lastRuntimeId, setLastRuntime } = useLastRuntime(); - const [acpCommand, setAcpCommand] = React.useState("buzz-acp"); - const [agentCommand, setAgentCommand] = React.useState("buzz-agent"); - const [agentArgs, setAgentArgs] = React.useState("acp"); - const [mcpCommand, setMcpCommand] = React.useState(""); - const [mcpToolsets, setMcpToolsets] = React.useState(""); - const prereqsQuery = useManagedAgentPrereqsQuery(acpCommand, mcpCommand, { - enabled: open, - }); - const [name, setName] = React.useState(""); - const [relayUrl, setRelayUrl] = React.useState(""); - const [spawnAfterCreate, setSpawnAfterCreate] = React.useState(true); - const [startOnAppLaunch, setStartOnAppLaunch] = React.useState(true); - const [turnTimeoutSeconds, setTurnTimeoutSeconds] = React.useState("320"); - const [parallelism, setParallelism] = React.useState("24"); - const [systemPrompt, setSystemPrompt] = React.useState(""); - const [envVars, setEnvVars] = React.useState({}); - const [selectedRuntimeId, setSelectedRuntimeId] = - React.useState("custom"); - const [hasSyncedProviderSelection, setHasSyncedProviderSelection] = - React.useState(false); - const [showAdvanced, setShowAdvanced] = React.useState(false); - const [respondTo, setRespondTo] = React.useState("owner-only"); - const [respondToAllowlist, setRespondToAllowlist] = React.useState( - [], - ); - - const [runOn, setRunOn] = React.useState<"local" | string>("local"); - const [providerConfig, setProviderConfig] = React.useState< - Record - >({}); - const [probedProvider, setProbedProvider] = - React.useState(null); - const [probeError, setProbeError] = React.useState(null); - - // Local-mode LLM provider and model — structured state for the credential - // gate and live discovery. Only rendered when the runtime supports provider - // selection (buzz-agent / goose). - const [provider, setProvider] = React.useState(""); - const [model, setModel] = React.useState(""); - const [isCustomProviderEditing, setIsCustomProviderEditing] = - React.useState(false); - const [isCustomModelEditing, setIsCustomModelEditing] = React.useState(false); - - // When `useMesh` is on, the agent runs buzz-agent against a member's - // shared compute. The ACP runtime + backend selectors are hidden; runtime - // fields are driven by `mesh_agent_preset(meshModelId)` and the submit - // input carries `model: meshModelId`. - const [useMesh, setUseMesh] = React.useState(false); - const [meshModelId, setMeshModelId] = React.useState(""); - const [meshTarget, setMeshTarget] = React.useState( - null, - ); - const [meshClientError, setMeshClientError] = React.useState( - null, - ); - // True while the relay-mesh client preflight (connect-request + dial) runs - // inside handleSubmit. That await can take tens of seconds and happens - // before createMutation fires, so isPending alone leaves the button live. - const [meshPreparing, setMeshPreparing] = React.useState(false); - - const runtimes = providersQuery.data ?? []; - const allProviders = allProvidersQuery.data ?? []; - const unavailableCount = allProviders.filter( - (p) => p.availability !== "available", - ).length; - const backendProviders = backendProvidersQuery.data ?? []; - const prereqs = prereqsQuery.data ?? null; - const selectedRuntime = React.useMemo( - () => runtimes.find((runtime) => runtime.id === selectedRuntimeId) ?? null, - [runtimes, selectedRuntimeId], - ); - const selectedBackendProvider = React.useMemo( - () => backendProviders.find((p) => p.id === runOn) ?? null, - [backendProviders, runOn], - ); - // Relay mesh always runs in local mode (buzz-agent + OpenAI-compat env); - // when on, it suppresses the backend "Run on" branch even if a stale - // `runOn` value remains. The relay-mesh path is its own thing. - const isProviderMode = !useMesh && runOn !== "local"; - - const isSpawnSupported = - prereqs?.acp.available === true && prereqs?.mcp.available === true; - const spawnToggleDisabled = - prereqsQuery.isLoading || (prereqs !== null && !isSpawnSupported); - const isDiscoveryPending = providersQuery.isLoading || prereqsQuery.isLoading; - - // Local mode: provider/model field visibility and live discovery. - // Create has no inherit checkbox — selectedRuntimeId IS the prospective runtime. - const llmProviderFieldVisible = - !isProviderMode && - !useMesh && - runtimeSupportsLlmProviderSelection(selectedRuntimeId); - const providerForDiscovery = llmProviderFieldVisible ? provider : ""; - - const { - discoveredModelOptions, - modelDiscoveryLoading, - modelDiscoveryStatus, - } = usePersonaModelDiscovery({ - envVars, - isCustomProviderEditing, - modelFieldVisible: llmProviderFieldVisible, - open, - provider: providerForDiscovery, - selectedRuntime: selectedRuntime ?? undefined, - }); - - // Full required credential key list for EnvVarsEditor amber locked rows — - // includes already-satisfied keys, not just missing ones. - const { data: runtimeFileConfig } = useRuntimeFileConfigQuery( - selectedRuntimeId, - { enabled: open }, - ); - const { data: bakedEnvKeys } = useBakedBuildEnvKeysQuery({ enabled: open }); - - // All required keys for the selected runtime + provider. - const allRequiredEnvKeys = React.useMemo( - () => - requiredCredentialEnvKeys( - selectedRuntimeId, - runtimeSupportsLlmProviderSelection(selectedRuntimeId) ? provider : "", - ), - [selectedRuntimeId, provider], - ); - - // Keys covered by the baked build env (Block-internal builds only) — silenced, - // produce no amber row or info row. - const bakedSatisfiedEnvKeys = React.useMemo( - () => getBakedSatisfiedEnvKeys(allRequiredEnvKeys, envVars, bakedEnvKeys), - [allRequiredEnvKeys, envVars, bakedEnvKeys], - ); - - // Credential keys satisfied by the runtime file config (e.g. goose config.yaml). - // These are shown as "Set in goose config" rows rather than amber required rows. - const fileSatisfiedEnvKeys = React.useMemo(() => { - if (!runtimeFileConfig) return [] as string[]; - return allRequiredEnvKeys.filter( - (key) => - (envVars[key] ?? "").length === 0 && - !bakedSatisfiedEnvKeys.includes(key) && - runtimeFileConfig.satisfiedEnvKeys.includes(key), - ); - }, [runtimeFileConfig, allRequiredEnvKeys, envVars, bakedSatisfiedEnvKeys]); - - const requiredEnvKeys = React.useMemo( - () => - allRequiredEnvKeys.filter( - (key) => - !bakedSatisfiedEnvKeys.includes(key) && - !fileSatisfiedEnvKeys.includes(key), - ), - [allRequiredEnvKeys, bakedSatisfiedEnvKeys, fileSatisfiedEnvKeys], - ); - - // Clear model when provider scope changes, mirroring AgentInstanceEditDialog. - React.useEffect(() => { - if ( - !open || - isCustomModelEditing || - !shouldClearKnownModelForSelectionScope({ - model, - provider: providerForDiscovery, - runtime: selectedRuntimeId, - }) - ) { - return; - } - setModel(""); - setIsCustomModelEditing(false); - }, [ - isCustomModelEditing, - model, - open, - providerForDiscovery, - selectedRuntimeId, - ]); - - React.useEffect(() => { - if (hasSyncedProviderSelection || providersQuery.isLoading) { - return; - } - - // Prefer last-used runtime from localStorage - const remembered = lastRuntimeId - ? runtimes.find((runtime) => runtime.id === lastRuntimeId) - : null; - if (remembered) { - setSelectedRuntimeId(remembered.id); - setAgentCommand(remembered.command); - setAgentArgs(remembered.defaultArgs.join(",")); - setMcpCommand(remembered.mcpCommand ?? ""); - } else { - const matchingProvider = - runtimes.find((runtime) => runtime.command === agentCommand) ?? null; - if (matchingProvider) { - setSelectedRuntimeId(matchingProvider.id); - } - } - setHasSyncedProviderSelection(true); - }, [ - agentCommand, - hasSyncedProviderSelection, - lastRuntimeId, - runtimes, - providersQuery.isLoading, - ]); - - React.useEffect(() => { - if ( - !prereqs || - (prereqs.acp.available && prereqs.mcp.available) || - !spawnAfterCreate - ) { - return; - } - - setSpawnAfterCreate(false); - }, [prereqs, spawnAfterCreate]); - - React.useEffect(() => { - if ( - providersQuery.error instanceof Error || - prereqsQuery.error instanceof Error - ) { - setShowAdvanced(true); - } - }, [prereqsQuery.error, providersQuery.error]); - - // Probe the backend provider when runOn changes to a non-local value - React.useEffect(() => { - if (!isProviderMode || !selectedBackendProvider) { - setProbedProvider(null); - setProbeError(null); - return; - } - - let cancelled = false; - setProbeError(null); - setProbedProvider(null); - - probeBackendProvider(selectedBackendProvider.binaryPath) - .then((result) => { - if (!cancelled) { - setProbedProvider(result); - // Initialize config from schema defaults so unchanged defaults - // are included in the submit payload (not silently dropped). - if (result.config_schema) { - const props = - (result.config_schema as Record)?.properties ?? - {}; - const defaults: Record = {}; - for (const [key, prop] of Object.entries(props) as [ - string, - Record, - ][]) { - if (prop.default != null) { - defaults[key] = String(prop.default); - } - } - setProviderConfig(defaults); - } - } - }) - .catch((err: unknown) => { - if (!cancelled) { - setProbeError(err instanceof Error ? err.message : String(err)); - } - }); - - return () => { - cancelled = true; - }; - }, [isProviderMode, selectedBackendProvider]); - - function reset() { - setName(""); - setRelayUrl(""); - setSpawnAfterCreate(true); - setStartOnAppLaunch(true); - setAcpCommand("buzz-acp"); - setAgentCommand("buzz-agent"); - setAgentArgs("acp"); - setMcpCommand(""); - setMcpToolsets(""); - setTurnTimeoutSeconds("320"); - setParallelism("24"); - setSystemPrompt(""); - setEnvVars({}); - setSelectedRuntimeId("custom"); - setHasSyncedProviderSelection(false); - setShowAdvanced(false); - setRunOn("local"); - setProviderConfig({}); - setProbedProvider(null); - setProbeError(null); - setProvider(""); - setModel(""); - setIsCustomProviderEditing(false); - setIsCustomModelEditing(false); - setUseMesh(false); - setMeshModelId(""); - setMeshClientError(null); - setRespondTo("owner-only"); - setRespondToAllowlist([]); - createMutation.reset(); - } - - function handleOpenChange(next: boolean) { - if (!next) { - reset(); - } - - onOpenChange(next); - } - - const selection: RuntimeModelProviderSelection = { - provider, - model, - isCustomProviderEditing, - isCustomModelEditing, - envVars, - }; - - function applySelection(next: RuntimeModelProviderSelection) { - setProvider(next.provider); - setModel(next.model); - setIsCustomProviderEditing(next.isCustomProviderEditing); - setIsCustomModelEditing(next.isCustomModelEditing); - setEnvVars(next.envVars); - } - - function handleProviderChange(nextProviderId: string) { - const previousRuntimeId = selectedRuntimeId; - setSelectedRuntimeId(nextProviderId); - - if (nextProviderId === "custom") { - setShowAdvanced(true); - } else { - const runtime = runtimes.find( - (candidate) => candidate.id === nextProviderId, - ); - if (runtime) { - setLastRuntime(nextProviderId); - setAgentCommand(runtime.command); - setAgentArgs(runtime.defaultArgs.join(",")); - setMcpCommand(runtime.mcpCommand ?? ""); - } - } - - applySelection( - selectionOnRuntimeChange(selection, { - previousRuntime: previousRuntimeId, - nextRuntime: nextProviderId, - nextRuntimeCanChooseProvider: - runtimeSupportsLlmProviderSelection(nextProviderId), - lockedRuntimeReset: "provider-only", - }), - ); - } - - function handleRunOnChange(value: string) { - setRunOn(value); - setProviderConfig({}); - setProbedProvider(null); - setProbeError(null); - } - - // Provider dropdown handler for local-mode provider field — mirrors Edit. - function handleProviderDropdownChange(nextValue: string) { - applySelection( - selectionOnProviderDropdownChange(selection, { - runtime: selectedRuntimeId, - nextValue, - clearModelWhenApiKeyMissing: false, - }), - ); - } - - // Check provider config required fields are filled. - const providerConfigComplete = React.useMemo(() => { - if (!isProviderMode || !probedProvider?.config_schema) return true; - const schema = probedProvider.config_schema as Record; - const required: string[] = (schema?.required as string[] | undefined) ?? []; - return required.every( - (key) => (providerConfig[key] ?? "").trim().length > 0, - ); - }, [isProviderMode, probedProvider, providerConfig]); - - // Allowlist mode requires at least one entry, mirroring the harness's own - // validation. If we let it through empty, the agent crash-loops at startup - // with a config error. - const respondToValid = - respondTo !== "allowlist" || respondToAllowlist.length > 0; - - const canSubmit = - name.trim().length > 0 && - !isDiscoveryPending && - !( - !isProviderMode && - spawnAfterCreate && - prereqs !== null && - !isSpawnSupported - ) && - // Block submission until probe succeeds in provider mode — required - // fields and config schema are only known after a successful probe. - !(isProviderMode && !probedProvider) && - providerConfigComplete && - // Relay-mesh mode requires a concrete serve target, not just a model name. - !(useMesh && (meshModelId.trim().length === 0 || meshTarget == null)) && - respondToValid && - !meshPreparing && - !createMutation.isPending; - - async function handleSubmit() { - setMeshClientError(null); - try { - if (useMesh) { - try { - if (!meshTarget) { - setMeshClientError( - "Select a relay mesh serve target before creating the agent.", - ); - return; - } - setMeshPreparing(true); - try { - await meshPrepareRelayMeshClient(meshModelId.trim(), meshTarget); - } finally { - setMeshPreparing(false); - } - } catch (err) { - setMeshClientError(err instanceof Error ? err.message : String(err)); - return; - } - } - - // Only send the allowlist when the mode is actually "allowlist". - // Other modes ignore it server-side, but keeping the wire clean makes - // the agent record easier to inspect. - const respondToFields = { - respondTo, - respondToAllowlist: - respondTo === "allowlist" ? respondToAllowlist : undefined, - } as const; - - const input: CreateManagedAgentInput = isProviderMode - ? { - name: name.trim(), - relayUrl: relayUrl.trim() || undefined, - turnTimeoutSeconds: - Number.parseInt(turnTimeoutSeconds, 10) > 0 - ? Number.parseInt(turnTimeoutSeconds, 10) - : undefined, - parallelism: - Number.parseInt(parallelism, 10) > 0 - ? Number.parseInt(parallelism, 10) - : undefined, - systemPrompt: systemPrompt.trim() || undefined, - envVars, - spawnAfterCreate: true, - startOnAppLaunch: false, // Remote agents don't auto-start with the desktop - backend: { - type: "provider", - id: runOn, - config: coerceConfigValues( - providerConfig, - probedProvider?.config_schema, - ), - }, - ...respondToFields, - } - : { - name: name.trim(), - relayUrl: relayUrl.trim() || undefined, - acpCommand: acpCommand.trim() || undefined, - agentCommand: agentCommand.trim() || undefined, - agentArgs: agentArgs - .split(",") - .map((value) => value.trim()) - .filter((value) => value.length > 0), - mcpCommand: mcpCommand.trim() || undefined, - mcpToolsets: mcpToolsets.trim() || undefined, - turnTimeoutSeconds: - Number.parseInt(turnTimeoutSeconds, 10) > 0 - ? Number.parseInt(turnTimeoutSeconds, 10) - : undefined, - parallelism: - Number.parseInt(parallelism, 10) > 0 - ? Number.parseInt(parallelism, 10) - : undefined, - systemPrompt: systemPrompt.trim() || undefined, - envVars, - model: useMesh - ? meshModelId.trim() || undefined - : model.trim() || undefined, - provider: !useMesh ? provider.trim() || undefined : undefined, - relayMesh: useMesh ? { modelRef: meshModelId.trim() } : undefined, - spawnAfterCreate, - // Relay-mesh agents need a freshly selected serve target to start; - // do not auto-restore them later with only the saved model/env. - startOnAppLaunch: useMesh ? false : startOnAppLaunch, - backend: { type: "local" }, - ...respondToFields, - }; - - const created = await createMutation.mutateAsync(input); - handleOpenChange(false); - onCreated(created); - } catch { - // React Query stores the error; keep the dialog open and render it inline. - } - } - - return ( - - -
- - Create agent - - This creates a local agent identity, syncs its display name when - possible, and can spawn `buzz-acp` immediately. - - - -
- - - v.trim()) - .filter((v) => v.length > 0), - mcpCommand, - model: meshModelId || null, - envVars, - }} - modelId={meshModelId} - targetEndpointAddr={meshTarget?.endpointAddr ?? ""} - onModelIdChange={(nextId, patch) => { - setMeshModelId(nextId); - if (patch == null) return; - // Fan out the preset into the existing setters so the rest - // of the dialog (and the submit branch) see normal local-mode - // values — relay-mesh is a curated local agent. - setAcpCommand(patch.acpCommand); - setAgentCommand(patch.agentCommand); - setAgentArgs(patch.agentArgs.join(",")); - setMcpCommand(patch.mcpCommand); - setEnvVars(patch.envVars); - }} - onTargetChange={setMeshTarget} - onUseMeshChange={(next) => { - setUseMesh(next); - if (!next) { - // Clearing the toggle: drop the model selection so the - // submit guard doesn't fire on a stale value. The runtime - // fields keep whatever the user had — they can re-pick - // ACP runtime or stay with the preset values, their call. - setMeshModelId(""); - setMeshTarget(null); - } - }} - useMesh={useMesh} - /> - - {/* Run on selector — only shown when backend providers are discovered */} - {!useMesh && backendProviders.length > 0 ? ( -
- - -
- ) : null} - - {/* Provider mode: trust warning + config fields */} - {isProviderMode && selectedBackendProvider ? ( -
-
- -

- This provider at{" "} - - {selectedBackendProvider.binaryPath} - {" "} - will receive your agent's private key. Only use - providers from trusted sources. -

-
- - {probeError ? ( -

- Could not probe provider: {probeError} -

- ) : null} - - {probedProvider?.config_schema ? ( - - ) : null} -
- ) : null} - - {/* Local mode: show the ACP runtime selector */} - {!isProviderMode && !useMesh ? ( - - ) : null} - - {/* Local mode: structured provider + model fields for credential - gate and live discovery. Only when the runtime supports it. */} - {llmProviderFieldVisible ? ( - - ) : null} - {llmProviderFieldVisible ? ( - - ) : null} - - { - setStartOnAppLaunch((current) => !current); - }} - onToggleSpawnAfterCreate={() => { - if (!spawnToggleDisabled) { - setSpawnAfterCreate((current) => !current); - } - }} - prereqs={prereqs} - startOnAppLaunch={isProviderMode ? false : startOnAppLaunch} - startOnAppLaunchDisabled={isProviderMode} - spawnAfterCreate={isProviderMode ? true : spawnAfterCreate} - spawnToggleDisabled={isProviderMode || spawnToggleDisabled} - /> - - - -
- - - {showAdvanced ? ( -
-
- - -

- Local Buzz binary checks and ACP runtime discovery now - live in Settings > Doctor. -

- - {providersQuery.error instanceof Error ? ( -

- {providersQuery.error.message} -

- ) : null} - - {prereqsQuery.error instanceof Error ? ( -

- {prereqsQuery.error.message} -

- ) : null} -
-
- ) : null} -
- - - - {meshClientError ? ( -

- Mesh client failed to start: {meshClientError} -

- ) : null} - - {createMutation.error instanceof Error ? ( -

- {createMutation.error.message} -

- ) : null} -
- -
- - -
-
- - - ); -} diff --git a/desktop/src/features/agents/ui/CreateAgentDialogSections.tsx b/desktop/src/features/agents/ui/CreateAgentDialogSections.tsx deleted file mode 100644 index f8be21c2ef..0000000000 --- a/desktop/src/features/agents/ui/CreateAgentDialogSections.tsx +++ /dev/null @@ -1,413 +0,0 @@ -import type { AcpRuntime, ManagedAgentPrereqs } from "@/shared/api/types"; -import { cn } from "@/shared/lib/cn"; -import { Input } from "@/shared/ui/input"; -import { Textarea } from "@/shared/ui/textarea"; -import { describeResolvedCommand } from "./agentUi"; - -export function CreateAgentBasicsFields({ - name, - onNameChange, -}: { - name: string; - onNameChange: (value: string) => void; -}) { - return ( -
- - onNameChange(event.target.value)} - placeholder="Support bot" - spellCheck={false} - value={name} - /> -

- Used as the local label and synced to the agent profile display name - when the relay accepts the create-time auth. -

-
- ); -} - -export function CreateAgentRuntimeField({ - runtimes, - runtimesLoading, - selectedRuntime, - selectedRuntimeId, - unavailableCount, - onRuntimeChange, -}: { - runtimes: AcpRuntime[]; - runtimesLoading: boolean; - selectedRuntime: AcpRuntime | null; - selectedRuntimeId: string; - unavailableCount: number; - onRuntimeChange: (value: string) => void; -}) { - return ( -
- - - {selectedRuntime ? ( -

- Detected via{" "} - - {describeResolvedCommand( - selectedRuntime.command, - selectedRuntime.binaryPath, - )} - -

- ) : runtimesLoading ? ( -

- Looking for installed ACP runtimes... -

- ) : ( -

- No known ACP runtime was detected. You can still enter a custom - command in Advanced setup. -

- )} - {unavailableCount > 0 ? ( -

- {unavailableCount} additional{" "} - {unavailableCount === 1 ? "runtime" : "runtimes"} available to - install. Visit Settings > Doctor to set{" "} - {unavailableCount === 1 ? "it" : "them"} up. -

- ) : null} -
- ); -} - -export function CreateAgentRuntimeFields({ - acpCommand, - agentArgs, - agentCommand, - mcpCommand, - mcpToolsets, - parallelism, - relayUrl, - selectedRuntimeId, - systemPrompt, - turnTimeoutSeconds, - onAcpCommandChange, - onAgentArgsChange, - onAgentCommandChange, - onMcpCommandChange, - onMcpToolsetsChange, - onParallelismChange, - onRelayUrlChange, - onSystemPromptChange, - onTurnTimeoutChange, -}: { - acpCommand: string; - agentArgs: string; - agentCommand: string; - mcpCommand: string; - mcpToolsets: string; - parallelism: string; - relayUrl: string; - selectedRuntimeId: string; - systemPrompt: string; - turnTimeoutSeconds: string; - onAcpCommandChange: (value: string) => void; - onAgentArgsChange: (value: string) => void; - onAgentCommandChange: (value: string) => void; - onMcpCommandChange: (value: string) => void; - onMcpToolsetsChange: (value: string) => void; - onParallelismChange: (value: string) => void; - onRelayUrlChange: (value: string) => void; - onSystemPromptChange: (value: string) => void; - onTurnTimeoutChange: (value: string) => void; -}) { - return ( - <> -
- {/* Relay URL pins the agent to a specific relay; blank falls back to - the active workspace relay. Shown for all agents. */} -
- - onRelayUrlChange(event.target.value)} - placeholder="Leave blank to use the workspace relay" - value={relayUrl} - /> -

- WebSocket URL of the relay this agent connects to. Leave blank to - use the active workspace relay. -

-
- -
- - onAcpCommandChange(event.target.value)} - value={acpCommand} - /> -

- The buzz-acp binary path or alias used to launch the ACP harness - process. -

-
-
- - {selectedRuntimeId === "custom" ? ( -
- - onAgentCommandChange(event.target.value)} - value={agentCommand} - /> -

- Full path or shell command for the agent binary when no known ACP - runtime was detected. -

-
- ) : null} - -
-
- - onAgentArgsChange(event.target.value)} - placeholder="Comma-separated" - value={agentArgs} - /> -

- buzz-acp splits args on commas, matching the testing guide. -

-
- -
- - onMcpCommandChange(event.target.value)} - value={mcpCommand} - /> -

- Optional. Only needed for agents that use a custom MCP server (e.g. - buzz-agent with buzz-dev-mcp). Leave blank for most agents. -

-
- -
- - onTurnTimeoutChange(event.target.value)} - placeholder="300" - value={turnTimeoutSeconds} - /> -

- Seconds before an agent turn is cancelled. Defaults to 300. -

-
- -
- - onParallelismChange(event.target.value)} - placeholder="1" - step="1" - type="number" - value={parallelism} - /> -

- Number of ACP worker subprocesses. buzz-acp allows 1-32. -

-
-
- -
- - onMcpToolsetsChange(event.target.value)} - placeholder="default,canvas,forums,dms,media" - value={mcpToolsets} - /> -

- Comma-separated list of toolsets to expose via BUZZ_TOOLSETS. - Available: default, channel_admin, dms, canvas, workflow_admin, - identity, forums, social, media. Leave blank for default toolsets - (default, canvas, forums, dms, media). -

-
- -
- -