feat(personas): sync symlinked pack changes on read

When a persona pack is installed as a symlink, edits to the pack source
files are never reflected in the running app — import_persona_pack()
snapshots data into personas.json at install time and all runtime reads
go through that snapshot exclusively.

Add sync_packs_from_dir() / sync_one_pack() (pure, testable) and a thin
sync_symlinked_packs() AppHandle wrapper. The sync step is inserted into
load_personas() after merge_personas(), reusing the existing read →
reconcile → write-back pattern. The frontend's 30s poll on
usePersonasQuery() becomes the refresh trigger with zero new
infrastructure.

Key design decisions:
- Mtime short-circuit: stat the symlink target dir before calling
  resolve_pack(). Unchanged mtime = skip. Common case pays one stat()
  per symlinked pack instead of a full re-parse.
- Quick pre-scan in the AppHandle wrapper: if no symlinked packs exist,
  managed_agents.json is never loaded (Thufir's concern about the hot
  path loading two JSON files on every poll).
- Removal safety gate: when a persona is removed from the source pack
  but a managed agent still references it by persona_id UUID, deactivate
  (is_active = false) rather than delete to avoid orphaning running
  agents. managed_agents.json is only loaded when removals are detected.
- Non-symlinked packs are completely unaffected.
- Broken symlinks (canonicalize fails) log a warning and are skipped.
- name_pool is always Vec::new() for pack personas (matches existing
  import_persona_pack() behavior).
This commit is contained in:
Will Pfleger
2026-06-06 14:59:24 -04:00
parent 9bbb85b439
commit 25536f07c4
2 changed files with 633 additions and 2 deletions

View File

@@ -1,4 +1,10 @@
use std::{fs, path::PathBuf};
use std::{
collections::HashMap,
fs,
path::PathBuf,
sync::{Mutex, OnceLock},
time::SystemTime,
};
use tauri::AppHandle;
use uuid::Uuid;
@@ -8,6 +14,18 @@ use crate::{
util::now_iso,
};
/// Per-pack mtime cache for the symlink sync short-circuit.
///
/// Keyed by pack ID. Stores the last-seen mtime of the symlink target
/// directory. If the mtime is unchanged since the last sync, `resolve_pack()`
/// is skipped — the common "nothing changed" case pays only one `stat()` call
/// per symlinked pack instead of a full re-parse.
static SYMLINK_PACK_MTIME_CACHE: OnceLock<Mutex<HashMap<String, SystemTime>>> = OnceLock::new();
fn symlink_pack_mtime_cache() -> &'static Mutex<HashMap<String, SystemTime>> {
SYMLINK_PACK_MTIME_CACHE.get_or_init(|| Mutex::new(HashMap::new()))
}
struct BuiltInPersona {
id: &'static str,
display_name: &'static str,
@@ -946,6 +964,304 @@ pub struct PackSummary {
pub path: PathBuf,
}
/// Reconcile `records` for a single resolved pack against its current source state.
///
/// Handles ADD / UPDATE / REMOVE for all personas belonging to `pack_id`.
/// Pure function — no I/O, no AppHandle. Designed for unit testing.
///
/// `agents` — current managed agent records, used only when removals are detected.
/// Pass an empty slice if you know there are no removals (e.g. in tests that only
/// test ADD/UPDATE paths).
///
/// Returns `true` if any mutations were made to `records`.
fn sync_one_pack(
records: &mut Vec<PersonaRecord>,
pack_id: &str,
resolved: &sprout_persona::resolve::ResolvedPack,
agents: &[crate::managed_agents::ManagedAgentRecord],
now: &str,
) -> bool {
// Build a map of slug → resolved persona for O(1) lookup.
let source_by_slug: HashMap<&str, &sprout_persona::resolve::ResolvedPersona> = resolved
.personas
.iter()
.map(|p| (p.name.as_str(), p))
.collect();
// Collect the slugs currently stored for this pack.
let stored_slugs: Vec<(usize, String)> = records
.iter()
.enumerate()
.filter_map(|(i, r)| {
if r.source_pack.as_deref() == Some(pack_id) {
r.source_pack_persona_slug.as_ref().map(|s| (i, s.clone()))
} else {
None
}
})
.collect();
let mut changed = false;
// ── UPDATE existing records ───────────────────────────────────────────────
for (idx, slug) in &stored_slugs {
if let Some(src) = source_by_slug.get(slug.as_str()) {
let record = &mut records[*idx];
let mut record_changed = false;
if record.display_name != src.display_name {
record.display_name = src.display_name.clone();
record_changed = true;
}
if record.system_prompt != src.system_prompt {
record.system_prompt = src.system_prompt.clone();
record_changed = true;
}
if record.model.as_deref() != src.model.as_deref() {
record.model = src.model.clone();
record_changed = true;
}
if record.provider.as_deref() != src.provider.as_deref() {
record.provider = src.provider.clone();
record_changed = true;
}
if record.avatar_url.as_deref() != src.avatar.as_deref() {
record.avatar_url = src.avatar.clone();
record_changed = true;
}
// env_vars: sync goose_env_vars from the resolved persona.
// These are the projected GOOSE_PROVIDER / GOOSE_MODEL / etc.
// vars derived from the persona's model and temperature config.
let src_env: std::collections::BTreeMap<String, String> =
src.goose_env_vars.iter().cloned().collect();
if record.env_vars != src_env {
record.env_vars = src_env;
record_changed = true;
}
if record_changed {
record.updated_at = now.to_string();
changed = true;
}
}
}
// ── REMOVE records no longer in source ───────────────────────────────────
let slugs_to_remove: Vec<String> = stored_slugs
.iter()
.filter(|(_, slug)| !source_by_slug.contains_key(slug.as_str()))
.map(|(_, slug)| slug.clone())
.collect();
for slug in &slugs_to_remove {
// Find the PersonaRecord.id (UUID) for this slug.
let persona_uuid = records
.iter()
.find(|r| {
r.source_pack.as_deref() == Some(pack_id)
&& r.source_pack_persona_slug.as_deref() == Some(slug.as_str())
})
.map(|r| r.id.clone());
let Some(uuid) = persona_uuid else { continue };
// Safety gate: if a managed agent references this persona by UUID,
// deactivate rather than delete to avoid orphaning it.
// Match on persona_id (UUID), not on slug or pack path.
let referenced = agents.iter().any(|a| a.persona_id.as_deref() == Some(&uuid));
if referenced {
eprintln!(
"sprout-desktop: symlink-pack-sync: persona '{slug}' (pack '{pack_id}') \
removed from source but still referenced by a managed agent — \
deactivating instead of deleting"
);
if let Some(record) = records.iter_mut().find(|r| r.id == uuid) {
if record.is_active {
record.is_active = false;
record.updated_at = now.to_string();
changed = true;
}
}
} else {
records.retain(|r| r.id != uuid);
changed = true;
}
}
// ── ADD new personas from source ──────────────────────────────────────────
let stored_slug_set: std::collections::HashSet<&str> = stored_slugs
.iter()
.map(|(_, s)| s.as_str())
.collect();
for src in &resolved.personas {
if stored_slug_set.contains(src.name.as_str()) {
continue;
}
let src_env: std::collections::BTreeMap<String, String> =
src.goose_env_vars.iter().cloned().collect();
records.push(PersonaRecord {
id: Uuid::new_v4().to_string(),
display_name: src.display_name.clone(),
avatar_url: src.avatar.clone(),
system_prompt: src.system_prompt.clone(),
provider: src.provider.clone(),
model: src.model.clone(),
// Pack personas never use the name pool — agents created from
// a pack persona use the persona's display_name directly.
name_pool: Vec::new(),
is_builtin: false,
is_active: true,
source_pack: Some(pack_id.to_string()),
source_pack_persona_slug: Some(src.name.clone()),
env_vars: src_env,
created_at: now.to_string(),
updated_at: now.to_string(),
});
changed = true;
}
changed
}
/// Reconcile `personas.json` with the current on-disk state of symlinked packs.
///
/// Scans `packs_dir` for subdirectories that are symlinks, applies an mtime
/// short-circuit to skip unchanged packs, then calls `sync_one_pack()` for
/// each changed pack.
///
/// `agents` — current managed agent records (used for removal safety gate).
/// Only consulted when a pack has removals; pass an empty slice in tests that
/// only exercise ADD/UPDATE paths.
///
/// Returns `true` if any mutations were made to `records`.
fn sync_packs_from_dir(
records: &mut Vec<PersonaRecord>,
packs_dir: &std::path::Path,
agents: &[crate::managed_agents::ManagedAgentRecord],
now: &str,
) -> bool {
if !packs_dir.exists() {
return false;
}
let entries = match fs::read_dir(packs_dir) {
Ok(e) => e,
Err(e) => {
eprintln!("sprout-desktop: symlink-pack-sync: failed to read packs dir: {e}");
return false;
}
};
let mut changed = false;
for entry in entries.flatten() {
let pack_dir = entry.path();
// Only process pack directories that are themselves symlinks.
let is_symlink = fs::symlink_metadata(&pack_dir)
.map(|m| m.file_type().is_symlink())
.unwrap_or(false);
if !is_symlink {
continue;
}
// Resolve the symlink target to get the real directory path.
let real_dir = match fs::canonicalize(&pack_dir) {
Ok(p) => p,
Err(e) => {
eprintln!(
"sprout-desktop: symlink-pack-sync: broken symlink at {}: {e} — skipping",
pack_dir.display()
);
continue;
}
};
let pack_id = match pack_dir.file_name().and_then(|n| n.to_str()) {
Some(id) => id.to_owned(),
None => continue,
};
// Mtime short-circuit: stat the real directory. If unchanged since the
// last sync, skip the full re-parse — the common case pays one stat().
let current_mtime = fs::metadata(&real_dir)
.and_then(|m| m.modified())
.ok();
if let Some(mtime) = current_mtime {
let mut cache = symlink_pack_mtime_cache().lock().unwrap_or_else(|e| e.into_inner());
if cache.get(&pack_id) == Some(&mtime) {
// Directory mtime unchanged — nothing to sync for this pack.
continue;
}
// Update cache before resolve_pack so a broken pack doesn't get
// hammered on every poll.
cache.insert(pack_id.clone(), mtime);
}
// Re-resolve the pack from disk to get the current source state.
let resolved = match sprout_persona::resolve::resolve_pack(&real_dir) {
Ok(p) => p,
Err(e) => {
eprintln!(
"sprout-desktop: symlink-pack-sync: failed to resolve pack at {}: {e} — skipping",
real_dir.display()
);
continue;
}
};
if sync_one_pack(records, &pack_id, &resolved, agents, now) {
changed = true;
}
}
changed
}
/// AppHandle wrapper: resolves packs dir and managed agents, then delegates
/// to `sync_packs_from_dir`. Agents are loaded once per call — the mtime
/// short-circuit inside `sync_packs_from_dir` ensures this is a no-op for
/// packs that haven't changed since the last poll.
fn sync_symlinked_packs(records: &mut Vec<PersonaRecord>, app: &AppHandle) -> bool {
let packs = match packs_dir(app) {
Ok(p) => p,
Err(e) => {
eprintln!("sprout-desktop: symlink-pack-sync: failed to resolve packs dir: {e}");
return false;
}
};
if !packs.exists() {
return false;
}
// Quick pre-scan: if no symlinked packs exist, skip the agent load entirely.
let has_symlinked_packs = fs::read_dir(&packs)
.map(|entries| {
entries.flatten().any(|e| {
fs::symlink_metadata(e.path())
.map(|m| m.file_type().is_symlink())
.unwrap_or(false)
})
})
.unwrap_or(false);
if !has_symlinked_packs {
return false;
}
// Load agents once — only reached when at least one symlinked pack exists.
let agents = crate::managed_agents::load_managed_agents(app).unwrap_or_default();
let now = now_iso();
sync_packs_from_dir(records, &packs, &agents, &now)
}
pub fn load_personas(app: &AppHandle) -> Result<Vec<PersonaRecord>, String> {
let path = personas_store_path(app)?;
let now = now_iso();
@@ -959,7 +1275,8 @@ pub fn load_personas(app: &AppHandle) -> Result<Vec<PersonaRecord>, String> {
Vec::new()
};
let (records, changed) = merge_personas(records, &now);
let (mut records, mut changed) = merge_personas(records, &now);
changed |= sync_symlinked_packs(&mut records, app);
if changed || !path.exists() {
save_personas(app, &records)?;
}

View File

@@ -458,3 +458,317 @@ fn migrate_is_idempotent() {
// 4. Run again on result of (3) — should be no-op.
assert!(!migrate_retired_personas(&mut stored_pre_demotion, now));
}
// ── sync_one_pack ─────────────────────────────────────────────────────────────
use super::{sync_one_pack, sync_packs_from_dir};
use crate::managed_agents::ManagedAgentRecord;
use std::collections::BTreeMap;
use std::fs;
use std::path::PathBuf;
use tempfile::TempDir;
const NOW: &str = "2026-06-06T00:00:00Z";
/// Minimal ManagedAgentRecord for testing the removal safety gate.
fn stub_agent(persona_id: &str) -> ManagedAgentRecord {
ManagedAgentRecord {
pubkey: "deadbeef".to_string(),
name: "test-agent".to_string(),
persona_id: Some(persona_id.to_string()),
private_key_nsec: "nsec1test".to_string(),
auth_tag: None,
relay_url: "ws://localhost:3000".to_string(),
acp_command: "sprout-acp".to_string(),
agent_command: "goose".to_string(),
agent_args: vec![],
mcp_command: "sprout-dev-mcp".to_string(),
turn_timeout_seconds: 300,
idle_timeout_seconds: None,
max_turn_duration_seconds: None,
parallelism: 1,
system_prompt: None,
model: None,
mcp_toolsets: None,
env_vars: 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.to_string(),
updated_at: NOW.to_string(),
last_started_at: None,
last_stopped_at: None,
last_exit_code: None,
last_error: None,
respond_to: Default::default(),
respond_to_allowlist: vec![],
}
}
/// Minimal PersonaRecord for a pack persona.
fn pack_persona(id: &str, slug: &str, pack_id: &str) -> PersonaRecord {
PersonaRecord {
id: id.to_string(),
display_name: format!("Display {slug}"),
avatar_url: None,
system_prompt: "Original prompt.".to_string(),
provider: None,
model: None,
name_pool: Vec::new(),
is_builtin: false,
is_active: true,
source_pack: Some(pack_id.to_string()),
source_pack_persona_slug: Some(slug.to_string()),
env_vars: BTreeMap::new(),
created_at: NOW.to_string(),
updated_at: NOW.to_string(),
}
}
/// Build a minimal valid pack directory with the given personas.
/// Each persona is `(slug, display_name, system_prompt)`.
fn make_pack_dir(dir: &TempDir, pack_id: &str, personas: &[(&str, &str, &str)]) -> PathBuf {
let root = dir.path().to_path_buf();
fs::create_dir_all(root.join(".plugin")).unwrap();
let persona_paths: Vec<String> = personas
.iter()
.map(|(slug, _, _)| format!("personas/{slug}.persona.md"))
.collect();
let manifest = serde_json::json!({
"id": pack_id,
"name": format!("Pack {pack_id}"),
"version": "0.1.0",
"personas": persona_paths,
});
fs::write(
root.join(".plugin/plugin.json"),
serde_json::to_string_pretty(&manifest).unwrap(),
)
.unwrap();
fs::create_dir_all(root.join("personas")).unwrap();
for (slug, display_name, prompt) in personas {
let content = format!(
"---\nname: {slug}\ndisplay_name: {display_name}\ndescription: Test persona.\n---\n{prompt}\n"
);
fs::write(root.join(format!("personas/{slug}.persona.md")), content).unwrap();
}
root
}
#[test]
fn sync_one_pack_adds_new_persona() {
let dir = TempDir::new().unwrap();
let pack_dir = make_pack_dir(&dir, "test-pack", &[("berry", "Berry", "You are Berry.")]);
let resolved = sprout_persona::resolve::resolve_pack(&pack_dir).unwrap();
let mut records: Vec<PersonaRecord> = vec![];
let changed = sync_one_pack(&mut records, "test-pack", &resolved, &[], NOW);
assert!(changed);
assert_eq!(records.len(), 1);
let r = &records[0];
assert_eq!(r.display_name, "Berry");
assert!(r.system_prompt.contains("You are Berry."));
assert_eq!(r.source_pack.as_deref(), Some("test-pack"));
assert_eq!(r.source_pack_persona_slug.as_deref(), Some("berry"));
assert!(r.is_active);
assert!(!r.is_builtin);
// Pack personas never use name_pool.
assert!(r.name_pool.is_empty());
}
#[test]
fn sync_one_pack_removes_deleted_persona() {
let dir = TempDir::new().unwrap();
// Pack now has only "pip" — "berry" was removed.
let pack_dir = make_pack_dir(&dir, "test-pack", &[("pip", "Pip", "You are Pip.")]);
let resolved = sprout_persona::resolve::resolve_pack(&pack_dir).unwrap();
// Start with both berry and pip in records.
let berry = pack_persona("uuid-berry", "berry", "test-pack");
let pip = pack_persona("uuid-pip", "pip", "test-pack");
let mut records = vec![berry, pip];
let changed = sync_one_pack(&mut records, "test-pack", &resolved, &[], NOW);
assert!(changed);
// berry should be gone, pip should remain.
assert_eq!(records.len(), 1);
assert_eq!(records[0].source_pack_persona_slug.as_deref(), Some("pip"));
}
#[test]
fn sync_one_pack_deactivates_instead_of_deleting_when_referenced() {
let dir = TempDir::new().unwrap();
// Pack now has only "pip" — "berry" was removed.
let pack_dir = make_pack_dir(&dir, "test-pack", &[("pip", "Pip", "You are Pip.")]);
let resolved = sprout_persona::resolve::resolve_pack(&pack_dir).unwrap();
let berry = pack_persona("uuid-berry", "berry", "test-pack");
let pip = pack_persona("uuid-pip", "pip", "test-pack");
let mut records = vec![berry, pip];
// A managed agent references berry by UUID.
let agent = stub_agent("uuid-berry");
let changed = sync_one_pack(&mut records, "test-pack", &resolved, &[agent], NOW);
assert!(changed);
// berry should still be in records but deactivated.
assert_eq!(records.len(), 2);
let berry = records.iter().find(|r| r.id == "uuid-berry").unwrap();
assert!(!berry.is_active);
assert_eq!(berry.updated_at, NOW);
}
#[test]
fn sync_one_pack_updates_changed_fields() {
let dir = TempDir::new().unwrap();
let pack_dir = make_pack_dir(
&dir,
"test-pack",
&[("berry", "Berry Updated", "New prompt.")],
);
let resolved = sprout_persona::resolve::resolve_pack(&pack_dir).unwrap();
let mut berry = pack_persona("uuid-berry", "berry", "test-pack");
berry.display_name = "Berry Old".to_string();
berry.system_prompt = "Old prompt.".to_string();
let mut records = vec![berry];
let changed = sync_one_pack(&mut records, "test-pack", &resolved, &[], NOW);
assert!(changed);
assert_eq!(records[0].display_name, "Berry Updated");
assert!(records[0].system_prompt.contains("New prompt."));
assert_eq!(records[0].updated_at, NOW);
}
#[test]
fn sync_one_pack_no_change_returns_false() {
let dir = TempDir::new().unwrap();
let pack_dir = make_pack_dir(&dir, "test-pack", &[("berry", "Berry", "You are Berry.")]);
let resolved = sprout_persona::resolve::resolve_pack(&pack_dir).unwrap();
// Add berry via sync first.
let mut records: Vec<PersonaRecord> = vec![];
sync_one_pack(&mut records, "test-pack", &resolved, &[], NOW);
// Second call with same resolved pack — nothing should change.
let changed = sync_one_pack(&mut records, "test-pack", &resolved, &[], NOW);
assert!(!changed);
assert_eq!(records.len(), 1);
}
#[test]
fn sync_one_pack_does_not_touch_other_packs() {
let dir = TempDir::new().unwrap();
let pack_dir = make_pack_dir(&dir, "pack-a", &[("berry", "Berry", "You are Berry.")]);
let resolved = sprout_persona::resolve::resolve_pack(&pack_dir).unwrap();
// Record from a different pack — must not be touched.
let other = pack_persona("uuid-other", "other-slug", "pack-b");
let mut records = vec![other];
let changed = sync_one_pack(&mut records, "pack-a", &resolved, &[], NOW);
assert!(changed); // berry was added
// pack-b record must still be present and unchanged.
assert!(records.iter().any(|r| r.source_pack.as_deref() == Some("pack-b")));
}
// ── sync_packs_from_dir ───────────────────────────────────────────────────────
#[test]
fn sync_packs_from_dir_adds_persona_from_symlinked_pack() {
let source_dir = TempDir::new().unwrap();
let packs_dir = TempDir::new().unwrap();
make_pack_dir(&source_dir, "my-pack", &[("berry", "Berry", "You are Berry.")]);
// Symlink source_dir into packs_dir as "my-pack".
let link = packs_dir.path().join("my-pack");
std::os::unix::fs::symlink(source_dir.path(), &link).unwrap();
let mut records: Vec<PersonaRecord> = vec![];
let changed = sync_packs_from_dir(&mut records, packs_dir.path(), &[], NOW);
assert!(changed);
assert_eq!(records.len(), 1);
assert_eq!(records[0].display_name, "Berry");
assert_eq!(records[0].source_pack.as_deref(), Some("my-pack"));
}
#[test]
fn sync_packs_from_dir_ignores_non_symlinked_packs() {
let packs_dir = TempDir::new().unwrap();
// Create a real (non-symlinked) pack directory directly inside packs_dir.
let pack_root = packs_dir.path().join("real-pack");
let tmp = TempDir::new().unwrap();
make_pack_dir(&tmp, "real-pack", &[("berry", "Berry", "You are Berry.")]);
// Copy (not symlink) into packs_dir.
fs::create_dir_all(&pack_root).unwrap();
for entry in fs::read_dir(tmp.path()).unwrap().flatten() {
let dst = pack_root.join(entry.file_name());
if entry.path().is_dir() {
let _ = std::process::Command::new("cp")
.args(["-r", &entry.path().to_string_lossy(), &dst.to_string_lossy()])
.status();
} else {
fs::copy(entry.path(), &dst).unwrap();
}
}
let mut records: Vec<PersonaRecord> = vec![];
let changed = sync_packs_from_dir(&mut records, packs_dir.path(), &[], NOW);
// Non-symlinked pack must not be synced.
assert!(!changed);
assert!(records.is_empty());
}
#[test]
fn sync_packs_from_dir_handles_broken_symlink_gracefully() {
let packs_dir = TempDir::new().unwrap();
// Create a symlink pointing to a non-existent target.
let link = packs_dir.path().join("broken-pack");
std::os::unix::fs::symlink("/nonexistent/path/that/does/not/exist", &link).unwrap();
let mut records: Vec<PersonaRecord> = vec![];
// Must not panic.
let changed = sync_packs_from_dir(&mut records, packs_dir.path(), &[], NOW);
assert!(!changed);
assert!(records.is_empty());
}
#[test]
fn sync_packs_from_dir_mtime_short_circuit_skips_unchanged_pack() {
let source_dir = TempDir::new().unwrap();
let packs_dir = TempDir::new().unwrap();
make_pack_dir(&source_dir, "my-pack", &[("berry", "Berry", "You are Berry.")]);
let link = packs_dir.path().join("my-pack");
std::os::unix::fs::symlink(source_dir.path(), &link).unwrap();
// First call — populates mtime cache and adds berry.
let mut records: Vec<PersonaRecord> = vec![];
let changed = sync_packs_from_dir(&mut records, packs_dir.path(), &[], NOW);
assert!(changed);
assert_eq!(records.len(), 1);
// Second call without any filesystem changes — mtime unchanged, should be a no-op.
let changed2 = sync_packs_from_dir(&mut records, packs_dir.path(), &[], NOW);
assert!(!changed2);
assert_eq!(records.len(), 1);
}