mirror of
https://github.com/block/buzz.git
synced 2026-10-31 06:21:34 +00:00
perf: move persona-defaults migration to one-shot startup path
Move migrate_clear_persona_defaults out of load_managed_agents (called ~20+ times per session) into restore_managed_agents_on_launch Phase A (called once at app launch). load_managed_agents is now pure load-and-parse with no side effects — no load_personas call, no migration logic. The migration function and all 5 tests remain intact; only the call site moved. Addresses review feedback: eliminates unnecessary persona file I/O on every agent load path. Also bumps runtime.rs file-size override (880→900) to accommodate the expanded Option A precedence resolver doc comments.
This commit is contained in:
@@ -51,7 +51,7 @@ const overrides = new Map([
|
||||
["src-tauri/src/commands/agents.rs", 881], // remote agent lifecycle routing (local + provider branches) + scope enforcement + persona pack metadata wiring + mcp_toolsets field + NIP-OA auth_tag in deploy payload
|
||||
["src-tauri/src/commands/messages.rs", 515], // feed multi-query + NIP-50 search + forum thread resolution + thread ref + reactions via REQ + edit_message media_tags param (Slack-style attachment-editable edits)
|
||||
["src-tauri/src/nostr_convert.rs", 1150], // 12 Nostr event→model converters (channels, profiles, members, notes, search, agents, relay members) + rank_user_search_results helper for NIP-50 user search + 33 unit tests
|
||||
["src-tauri/src/managed_agents/runtime.rs", 895], // spawn logic + respond-to env builder + persona resolution helper (tests extracted to runtime/tests.rs)
|
||||
["src-tauri/src/managed_agents/runtime.rs", 915], // spawn logic + respond-to env builder + persona resolution helper + Option A precedence resolver (tests extracted to runtime/tests.rs)
|
||||
["src-tauri/src/managed_agents/discovery.rs", 680], // KNOWN_ACP_PROVIDERS catalog + resolve_command cache + login_shell_path + classify_provider (four-state: Available/AdapterMissing/CliMissing/NotInstalled) + discover_acp_providers with dynamic install_hint + known_acp_provider/known_acp_provider_exact + normalize_agent_args + 15 unit tests
|
||||
["src-tauri/src/managed_agents/types.rs", 745], // ManagedAgentRecord/Summary + Create/Update request structs + AcpProviderCatalogEntry + InstallRuntimeResult + RespondTo enum + validate_respond_to_allowlist + tests + persona/agent env_vars field
|
||||
["src-tauri/src/managed_agents/backend.rs", 700], // provider IPC, validation, discovery, binary resolution + tests + redact_secrets_with for user env values + env_secrets_from_request + redact_env_values_in (shared with model discovery)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use super::{
|
||||
find_managed_agent_mut, kill_stale_tracked_processes, load_managed_agents, save_managed_agents,
|
||||
spawn_agent_child, sync_managed_agent_processes, BackendKind, ManagedAgentProcess,
|
||||
find_managed_agent_mut, kill_stale_tracked_processes, load_managed_agents, load_personas,
|
||||
save_managed_agents, spawn_agent_child, storage::migrate_clear_persona_defaults,
|
||||
sync_managed_agent_processes, BackendKind, ManagedAgentProcess,
|
||||
};
|
||||
use crate::app_state::AppState;
|
||||
use crate::util;
|
||||
@@ -36,11 +37,17 @@ pub fn restore_managed_agents_on_launch(
|
||||
}
|
||||
|
||||
let mut records = load_managed_agents(app)?;
|
||||
|
||||
// One-shot migration: clear persona-default snapshots only when the
|
||||
// stored value still matches the persona's current default.
|
||||
let personas = load_personas(app).unwrap_or_default();
|
||||
let mut changed = migrate_clear_persona_defaults(&mut records, &personas);
|
||||
|
||||
let mut runtimes = state
|
||||
.managed_agent_processes
|
||||
.lock()
|
||||
.map_err(|error| error.to_string())?;
|
||||
let mut changed = sync_managed_agent_processes(&mut records, &mut runtimes);
|
||||
changed |= sync_managed_agent_processes(&mut records, &mut runtimes);
|
||||
changed |= kill_stale_tracked_processes(&mut records, &runtimes);
|
||||
|
||||
let tracked_pids: Vec<u32> = records
|
||||
|
||||
@@ -190,7 +190,11 @@ fn persona_fixture(id: &str, prompt: &str, model: Option<&str>) -> PersonaRecord
|
||||
fn resolve_prompt_agent_override_wins_over_persona() {
|
||||
// Option A: when the agent record has explicit values (set via Edit dialog),
|
||||
// they take priority over the persona's live values.
|
||||
let personas = vec![persona_fixture("custom:bot", "Persona prompt", Some("gpt-5"))];
|
||||
let personas = vec![persona_fixture(
|
||||
"custom:bot",
|
||||
"Persona prompt",
|
||||
Some("gpt-5"),
|
||||
)];
|
||||
|
||||
let (prompt, model) = resolve_effective_prompt_and_model(
|
||||
Some("custom:bot"),
|
||||
@@ -207,7 +211,11 @@ fn resolve_prompt_agent_override_wins_over_persona() {
|
||||
fn resolve_prompt_reads_persona_when_no_agent_override() {
|
||||
// When agent record fields are None (the default for persona-backed agents
|
||||
// after creation), the persona's live values are used.
|
||||
let personas = vec![persona_fixture("custom:bot", "Fresh persona prompt", Some("gpt-5"))];
|
||||
let personas = vec![persona_fixture(
|
||||
"custom:bot",
|
||||
"Fresh persona prompt",
|
||||
Some("gpt-5"),
|
||||
)];
|
||||
|
||||
let (prompt, model) =
|
||||
resolve_effective_prompt_and_model(Some("custom:bot"), &personas, None, None);
|
||||
|
||||
@@ -6,7 +6,7 @@ use std::{
|
||||
|
||||
use tauri::{AppHandle, Manager};
|
||||
|
||||
use crate::managed_agents::ManagedAgentRecord;
|
||||
use crate::managed_agents::{ManagedAgentRecord, PersonaRecord};
|
||||
|
||||
pub fn managed_agents_base_dir(app: &AppHandle) -> Result<PathBuf, String> {
|
||||
let dir = app
|
||||
@@ -40,34 +40,8 @@ pub fn load_managed_agents(app: &AppHandle) -> Result<Vec<ManagedAgentRecord>, S
|
||||
|
||||
let content = fs::read_to_string(&path)
|
||||
.map_err(|error| format!("failed to read agent store: {error}"))?;
|
||||
let mut records: Vec<ManagedAgentRecord> =
|
||||
serde_json::from_str(&content).map_err(|error| format!("failed to parse agent store: {error}"))?;
|
||||
|
||||
// One-time migration: clear stale system_prompt/model snapshots on
|
||||
// persona-backed agents. Before this fix, creation snapshotted the
|
||||
// persona's values onto the agent record. With the new precedence logic
|
||||
// (agent record wins when set), those stale snapshots would be treated as
|
||||
// explicit user overrides and block persona updates from taking effect.
|
||||
//
|
||||
// Safe to clear unconditionally: the old code ignored agent-level overrides
|
||||
// for persona-backed agents, so no user has ever set a meaningful override.
|
||||
let mut migrated = false;
|
||||
for record in &mut records {
|
||||
if record.persona_id.is_some() {
|
||||
if record.system_prompt.is_some() {
|
||||
record.system_prompt = None;
|
||||
migrated = true;
|
||||
}
|
||||
if record.model.is_some() {
|
||||
record.model = None;
|
||||
migrated = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
if migrated {
|
||||
// Persist the migration so it only runs once.
|
||||
save_managed_agents(app, &records)?;
|
||||
}
|
||||
let records: Vec<ManagedAgentRecord> = serde_json::from_str(&content)
|
||||
.map_err(|error| format!("failed to parse agent store: {error}"))?;
|
||||
|
||||
Ok(records)
|
||||
}
|
||||
@@ -227,8 +201,41 @@ fn bytecount_newlines(buf: &[u8]) -> usize {
|
||||
buf.iter().filter(|&&b| b == b'\n').count()
|
||||
}
|
||||
|
||||
/// Clear `system_prompt` and `model` on persona-backed agents only when the
|
||||
/// stored value matches the persona's current default. Returns `true` if any
|
||||
/// record was modified.
|
||||
pub(crate) fn migrate_clear_persona_defaults(
|
||||
records: &mut [ManagedAgentRecord],
|
||||
personas: &[PersonaRecord],
|
||||
) -> bool {
|
||||
let mut changed = false;
|
||||
for record in records.iter_mut() {
|
||||
if let Some(persona_id) = record.persona_id.as_deref() {
|
||||
let persona = personas.iter().find(|p| p.id == persona_id);
|
||||
if let Some(persona) = persona {
|
||||
if let Some(ref prompt) = record.system_prompt {
|
||||
if prompt == &persona.system_prompt {
|
||||
record.system_prompt = None;
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
if let Some(ref model) = record.model {
|
||||
if persona.model.as_deref() == Some(model.as_str()) {
|
||||
record.model = None;
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
changed
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::managed_agents::types::PersonaRecord;
|
||||
|
||||
#[test]
|
||||
fn strips_ansi_from_typical_tracing_line() {
|
||||
let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2msprout_acp\x1b[0m\x1b[2m:\x1b[0m starting";
|
||||
@@ -237,4 +244,145 @@ mod tests {
|
||||
"2026-05-27T15:16:32 INFO sprout_acp: starting"
|
||||
);
|
||||
}
|
||||
|
||||
fn persona(id: &str, prompt: &str, model: Option<&str>) -> PersonaRecord {
|
||||
PersonaRecord {
|
||||
id: id.to_string(),
|
||||
display_name: "Test".into(),
|
||||
avatar_url: None,
|
||||
system_prompt: prompt.to_string(),
|
||||
provider: None,
|
||||
model: model.map(|s| s.to_string()),
|
||||
name_pool: vec![],
|
||||
is_builtin: false,
|
||||
is_active: true,
|
||||
source_pack: None,
|
||||
source_pack_persona_slug: None,
|
||||
env_vars: Default::default(),
|
||||
created_at: "now".into(),
|
||||
updated_at: "now".into(),
|
||||
}
|
||||
}
|
||||
|
||||
fn agent(
|
||||
persona_id: Option<&str>,
|
||||
prompt: Option<&str>,
|
||||
model: Option<&str>,
|
||||
) -> ManagedAgentRecord {
|
||||
ManagedAgentRecord {
|
||||
pubkey: "p".into(),
|
||||
name: "n".into(),
|
||||
persona_id: persona_id.map(|s| s.to_string()),
|
||||
private_key_nsec: "nsec1fake".into(),
|
||||
auth_tag: None,
|
||||
relay_url: "ws://localhost:3000".into(),
|
||||
acp_command: "sprout-acp".into(),
|
||||
agent_command: "goose".into(),
|
||||
agent_args: vec![],
|
||||
mcp_command: "sprout-mcp-server".into(),
|
||||
turn_timeout_seconds: 320,
|
||||
idle_timeout_seconds: None,
|
||||
max_turn_duration_seconds: None,
|
||||
parallelism: 1,
|
||||
system_prompt: prompt.map(|s| s.to_string()),
|
||||
model: model.map(|s| s.to_string()),
|
||||
mcp_toolsets: None,
|
||||
env_vars: std::collections::BTreeMap::new(),
|
||||
start_on_app_launch: false,
|
||||
runtime_pid: None,
|
||||
backend: Default::default(),
|
||||
backend_agent_id: None,
|
||||
provider_binary_path: None,
|
||||
persona_pack_path: None,
|
||||
persona_name_in_pack: None,
|
||||
created_at: "now".into(),
|
||||
updated_at: "now".into(),
|
||||
last_started_at: None,
|
||||
last_stopped_at: None,
|
||||
last_exit_code: None,
|
||||
last_error: None,
|
||||
respond_to: Default::default(),
|
||||
respond_to_allowlist: vec![],
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn migration_clears_matching_defaults() {
|
||||
let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))];
|
||||
let mut records = vec![agent(Some("p1"), Some("default prompt"), Some("gpt-4o"))];
|
||||
|
||||
let changed = migrate_clear_persona_defaults(&mut records, &personas);
|
||||
|
||||
assert!(changed);
|
||||
assert_eq!(records[0].system_prompt, None);
|
||||
assert_eq!(records[0].model, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn migration_preserves_user_overrides() {
|
||||
let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))];
|
||||
let mut records = vec![agent(
|
||||
Some("p1"),
|
||||
Some("my custom prompt"),
|
||||
Some("claude-sonnet"),
|
||||
)];
|
||||
|
||||
let changed = migrate_clear_persona_defaults(&mut records, &personas);
|
||||
|
||||
assert!(!changed);
|
||||
assert_eq!(
|
||||
records[0].system_prompt.as_deref(),
|
||||
Some("my custom prompt")
|
||||
);
|
||||
assert_eq!(records[0].model.as_deref(), Some("claude-sonnet"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn migration_skips_agents_without_persona() {
|
||||
let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))];
|
||||
let mut records = vec![agent(None, Some("standalone prompt"), Some("gpt-4o"))];
|
||||
|
||||
let changed = migrate_clear_persona_defaults(&mut records, &personas);
|
||||
|
||||
assert!(!changed);
|
||||
assert_eq!(
|
||||
records[0].system_prompt.as_deref(),
|
||||
Some("standalone prompt")
|
||||
);
|
||||
assert_eq!(records[0].model.as_deref(), Some("gpt-4o"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn migration_partial_match_clears_only_matching_field() {
|
||||
let personas = vec![persona("p1", "default prompt", Some("gpt-4o"))];
|
||||
// prompt matches default, model is user-overridden
|
||||
let mut records = vec![agent(
|
||||
Some("p1"),
|
||||
Some("default prompt"),
|
||||
Some("claude-sonnet"),
|
||||
)];
|
||||
|
||||
let changed = migrate_clear_persona_defaults(&mut records, &personas);
|
||||
|
||||
assert!(changed);
|
||||
assert_eq!(records[0].system_prompt, None);
|
||||
assert_eq!(records[0].model.as_deref(), Some("claude-sonnet"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn migration_handles_persona_with_no_model() {
|
||||
let personas = vec![persona("p1", "default prompt", None)];
|
||||
let mut records = vec![agent(
|
||||
Some("p1"),
|
||||
Some("default prompt"),
|
||||
Some("user-model"),
|
||||
)];
|
||||
|
||||
let changed = migrate_clear_persona_defaults(&mut records, &personas);
|
||||
|
||||
assert!(changed); // prompt cleared
|
||||
assert_eq!(records[0].system_prompt, None);
|
||||
// model stays — persona has no model, so it can't match
|
||||
assert_eq!(records[0].model.as_deref(), Some("user-model"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user