fix(desktop): preserve gated Bestie identity

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
This commit is contained in:
Arjun Mahanti
2026-08-26 19:53:33 -04:00
committed by Fizz
parent 9dfa37fd4c
commit 38115b7aec
7 changed files with 254 additions and 30 deletions

View File

@@ -190,7 +190,13 @@ pub(crate) fn built_in_persona_definition(id: &str, now: &str) -> Option<AgentDe
}
fn built_in_order(id: &str) -> Option<usize> {
available_built_in_personas(bestie_build_enabled()).position(|persona| persona.id == id)
BUILT_IN_PERSONAS
.iter()
.position(|persona| persona.id == id)
}
fn persona_available_in_build(id: &str, include_bestie: bool) -> bool {
include_bestie || id != BESTIE_PERSONA_ID
}
fn sort_personas(records: &mut [AgentDefinition]) {
@@ -215,10 +221,19 @@ fn sort_personas(records: &mut [AgentDefinition]) {
});
}
fn merge_personas(mut stored: Vec<AgentDefinition>, now: &str) -> (Vec<AgentDefinition>, bool) {
#[cfg(test)]
fn merge_personas(stored: Vec<AgentDefinition>, now: &str) -> (Vec<AgentDefinition>, bool) {
merge_personas_for_build(stored, now, bestie_build_enabled())
}
fn merge_personas_for_build(
mut stored: Vec<AgentDefinition>,
now: &str,
include_bestie: bool,
) -> (Vec<AgentDefinition>, bool) {
let mut changed = false;
for built_in in built_in_persona_records(now) {
for built_in in built_in_persona_records_for_build(now, include_bestie) {
if let Some(existing) = stored.iter_mut().find(|record| record.id == built_in.id) {
if !existing.is_builtin {
existing.is_builtin = true;
@@ -232,6 +247,8 @@ fn merge_personas(mut stored: Vec<AgentDefinition>, now: &str) -> (Vec<AgentDefi
// Demote any stored persona still flagged as built-in whose id is no
// longer in BUILT_IN_PERSONAS (e.g. a built-in that has been retired).
// Build-gated personas remain known built-ins even when unavailable. An
// ineligible build hides them but must never rewrite their ownership.
// The record stays so existing managed-agent and team references keep
// working; the user can delete it from the catalog like any custom
// persona once they no longer need it.
@@ -254,6 +271,34 @@ fn merge_personas(mut stored: Vec<AgentDefinition>, now: &str) -> (Vec<AgentDefi
(stored, changed)
}
fn visible_personas_for_build(
records: Vec<AgentDefinition>,
include_bestie: bool,
) -> Vec<AgentDefinition> {
records
.into_iter()
.filter(|record| persona_available_in_build(&record.id, include_bestie))
.collect()
}
fn definitions_for_save(
records: &[AgentDefinition],
existing: &[AgentDefinition],
include_bestie: bool,
) -> Vec<AgentDefinition> {
let mut complete = records.to_vec();
for hidden_builtin in existing.iter().filter(|record| {
built_in_order(&record.id).is_some()
&& !persona_available_in_build(&record.id, include_bestie)
}) {
if !complete.iter().any(|record| record.id == hidden_builtin.id) {
complete.push(hidden_builtin.clone());
}
}
sort_personas(&mut complete);
complete
}
/// Soft-deprecate retired built-in personas by appending " (retired)" to
/// their display name and marking them inactive. Never removes records —
/// the cost is extra records for pre-transition users, but this
@@ -375,6 +420,7 @@ pub fn load_personas<R: tauri::Runtime>(
app: &AppHandle<R>,
) -> Result<Vec<AgentDefinition>, String> {
let now = now_iso();
let include_bestie = bestie_build_enabled();
// Post-fold: definitions live in the unified agent store, presented in
// the legacy shape. Pre-fold stores are converted by
@@ -385,12 +431,12 @@ pub fn load_personas<R: tauri::Runtime>(
.filter_map(|record| record.to_definition_view())
.collect();
let (records, changed) = merge_personas(records, &now);
let (records, changed) = merge_personas_for_build(records, &now, include_bestie);
if changed {
save_personas(app, &records)?;
save_personas_for_build(app, &records, include_bestie)?;
}
Ok(records)
Ok(visible_personas_for_build(records, include_bestie))
}
/// Read the raw persona records at `path` — no built-in merge, no write-back.
@@ -415,12 +461,23 @@ pub fn save_personas<R: tauri::Runtime>(
app: &AppHandle<R>,
records: &[AgentDefinition],
) -> Result<(), String> {
let mut sorted = records.to_vec();
sort_personas(&mut sorted);
save_personas_for_build(app, records, bestie_build_enabled())
}
fn save_personas_for_build<R: tauri::Runtime>(
app: &AppHandle<R>,
records: &[AgentDefinition],
include_bestie: bool,
) -> Result<(), String> {
let existing: Vec<_> = crate::managed_agents::storage::load_agent_definitions_unfiltered(app)?
.iter()
.filter_map(|record| record.to_definition_view())
.collect();
let complete = definitions_for_save(records, &existing, include_bestie);
// Post-fold: persona saves write key-less definition records into the
// unified agent store (instances preserved by `save_agent_definitions`).
let definitions: Vec<_> = sorted
let definitions: Vec<_> = complete
.into_iter()
.map(|persona| persona.into_agent_record())
.collect();

View File

@@ -1,8 +1,8 @@
use super::{
built_in_persona_records, built_in_persona_records_for_build, ensure_persona_ids_are_active,
ensure_persona_is_active, merge_personas, migrate_retired_personas,
validate_persona_activation_change, validate_persona_deletion, BUILT_IN_PERSONAS,
RETIRED_PERSONAS,
built_in_persona_records, built_in_persona_records_for_build, definitions_for_save,
ensure_persona_ids_are_active, ensure_persona_is_active, merge_personas,
merge_personas_for_build, migrate_retired_personas, validate_persona_activation_change,
validate_persona_deletion, visible_personas_for_build, BUILT_IN_PERSONAS, RETIRED_PERSONAS,
};
use crate::managed_agents::discovery::{default_agent_command, effective_agent_command};
use crate::managed_agents::AgentDefinition;
@@ -76,6 +76,120 @@ fn bestie_persona_requires_the_internal_build_capability() {
assert!(bestie.is_active);
}
#[test]
fn ineligible_build_hides_bestie_without_demoting_it() {
let now = "2026-03-19T00:00:00Z";
let (eligible, _) = merge_personas_for_build(Vec::new(), now, true);
let original = eligible
.iter()
.find(|record| record.id == "builtin:bestie")
.expect("eligible build should seed Bestie")
.clone();
let (ineligible, changed) = merge_personas_for_build(eligible, now, false);
let persisted = ineligible
.iter()
.find(|record| record.id == "builtin:bestie")
.expect("ineligible builds must preserve the durable Bestie definition");
assert!(
!changed,
"changing build eligibility must not rewrite the store"
);
assert!(persisted.is_builtin);
assert_eq!(
serde_json::to_value(persisted).unwrap(),
serde_json::to_value(original).unwrap()
);
assert!(!visible_personas_for_build(ineligible, false)
.iter()
.any(|record| record.id == "builtin:bestie"));
}
#[test]
fn unified_store_preserves_bestie_and_instance_across_build_transitions() {
let dir = tempfile::tempdir().expect("temp dir");
let path = dir.path().join("managed-agents.json");
let now = "2026-03-19T00:00:00Z";
let (eligible, _) = merge_personas_for_build(Vec::new(), now, true);
let original_bestie = eligible
.iter()
.find(|record| record.id == "builtin:bestie")
.expect("eligible build should seed Bestie")
.clone();
let mut instance = original_bestie.clone().into_agent_record();
instance.pubkey = "a".repeat(64);
instance.name = "My Bestie".to_string();
instance.persona_id = Some("builtin:bestie".to_string());
instance.slug = None;
instance.display_name = None;
instance.relay_url = "wss://buzz.example".to_string();
crate::managed_agents::storage::write_agent_store_to_path(
&path,
eligible
.iter()
.cloned()
.map(AgentDefinition::into_agent_record)
.collect(),
vec![instance.clone()],
)
.expect("write eligible unified store");
let raw = crate::managed_agents::storage::load_agent_store_from_path(&path)
.expect("load ineligible unified store");
let existing: Vec<_> = raw
.iter()
.filter(|record| record.pubkey.is_empty())
.filter_map(|record| record.to_definition_view())
.collect();
let (all_ineligible, changed) = merge_personas_for_build(existing.clone(), now, false);
assert!(
!changed,
"ineligible load must not mutate durable ownership"
);
let visible = visible_personas_for_build(all_ineligible, false);
assert!(!visible.iter().any(|record| record.id == "builtin:bestie"));
let definitions = definitions_for_save(&visible, &existing, false)
.into_iter()
.map(AgentDefinition::into_agent_record)
.collect::<Vec<_>>();
crate::managed_agents::storage::save_agent_definitions_to_path(&path, &definitions)
.expect("save from ineligible build");
let after_save = crate::managed_agents::storage::load_agent_store_from_path(&path)
.expect("reload unified store");
let saved_bestie = after_save
.iter()
.find(|record| record.slug.as_deref() == Some("builtin:bestie"))
.and_then(|record| record.to_definition_view())
.expect("hidden Bestie definition should survive the save");
assert!(saved_bestie.is_builtin);
assert_eq!(
serde_json::to_value(saved_bestie).unwrap(),
serde_json::to_value(original_bestie).unwrap()
);
assert_eq!(
after_save
.iter()
.find(|record| record.pubkey == instance.pubkey)
.and_then(|record| record.persona_id.as_deref()),
Some("builtin:bestie"),
"definition saves must preserve keyed instance references"
);
let reloaded_definitions = after_save
.iter()
.filter(|record| record.pubkey.is_empty())
.filter_map(|record| record.to_definition_view())
.collect();
let (eligible_again, _) = merge_personas_for_build(reloaded_definitions, now, true);
assert!(visible_personas_for_build(eligible_again, true)
.iter()
.any(|record| record.id == "builtin:bestie" && record.is_builtin));
}
#[test]
fn merge_personas_preserves_custom_records() {
let custom = custom_persona("custom:test", "Custom");

View File

@@ -242,12 +242,16 @@ fn load_agent_store<R: tauri::Runtime>(
app: &AppHandle<R>,
) -> Result<Vec<ManagedAgentRecord>, String> {
let path = managed_agents_store_path(app)?;
load_agent_store_from_path(&path)
}
pub(crate) fn load_agent_store_from_path(path: &Path) -> Result<Vec<ManagedAgentRecord>, String> {
if !path.exists() {
return Ok(Vec::new());
}
let content = fs::read_to_string(&path)
.map_err(|error| format!("failed to read agent store: {error}"))?;
let content =
fs::read_to_string(path).map_err(|error| format!("failed to read agent store: {error}"))?;
serde_json::from_str(&content).map_err(|error| {
// Fail loudly and preserve the evidence: a later in-app save rewrites
// this file wholesale, which would silently destroy a malformed hand
@@ -255,7 +259,7 @@ fn load_agent_store<R: tauri::Runtime>(
// reconcile): the broken content survives as `.invalid` for the user
// to recover, and the parse error propagates instead of being
// swallowed into an empty store.
backup_invalid_store(&path);
backup_invalid_store(path);
format!("failed to parse agent store (preserved as .invalid): {error}")
})
}
@@ -398,11 +402,19 @@ pub(crate) fn save_agent_definitions<R: tauri::Runtime>(
app: &AppHandle<R>,
definitions: &[ManagedAgentRecord],
) -> Result<(), String> {
let mut instances = load_agent_store(app)?;
let path = managed_agents_store_path(app)?;
save_agent_definitions_to_path(&path, definitions)
}
pub(crate) fn save_agent_definitions_to_path(
path: &Path,
definitions: &[ManagedAgentRecord],
) -> Result<(), String> {
let mut instances = load_agent_store_from_path(path)?;
instances.retain(|record| !record.pubkey.is_empty());
let mut definitions = definitions.to_vec();
definitions.retain(|record| record.pubkey.is_empty());
write_agent_store(app, definitions, instances)
write_agent_store_to_path(path, definitions, instances)
}
/// Serialize definitions + instances into the single unified store file.
@@ -410,6 +422,15 @@ pub(crate) fn save_agent_definitions<R: tauri::Runtime>(
/// name/pubkey order their save path established.
fn write_agent_store<R: tauri::Runtime>(
app: &AppHandle<R>,
definitions: Vec<ManagedAgentRecord>,
instances: Vec<ManagedAgentRecord>,
) -> Result<(), String> {
let path = managed_agents_store_path(app)?;
write_agent_store_to_path(&path, definitions, instances)
}
pub(crate) fn write_agent_store_to_path(
path: &Path,
mut definitions: Vec<ManagedAgentRecord>,
instances: Vec<ManagedAgentRecord>,
) -> Result<(), String> {
@@ -417,7 +438,6 @@ fn write_agent_store<R: tauri::Runtime>(
let mut all = definitions;
all.extend(instances);
let path = managed_agents_store_path(app)?;
let payload = serde_json::to_vec_pretty(&all)
.map_err(|error| format!("failed to serialize agent store: {error}"))?;
@@ -425,7 +445,7 @@ fn write_agent_store<R: tauri::Runtime>(
// fallback. Write it owner-only (`0o600`) unconditionally — harmless for the
// keyring-backed case (it is the user's own agent store) and closes the
// umask window a post-write `chmod` would leave open.
atomic_write_json_restricted(&path, &payload)
atomic_write_json_restricted(path, &payload)
}
/// Write each record's in-memory key to the keyring and blank the inline copy

View File

@@ -22,7 +22,7 @@ test("prefers the built-in Bestie over a name fallback", () => {
personaId: "builtin:bestie",
});
assert.equal(pickBestieAgent([chief, bestie]), bestie);
assert.equal(pickBestieAgent([chief, bestie], "wss://buzz.example"), bestie);
});
test("reuses an existing Chief of Staff within the active relay", () => {
@@ -52,5 +52,36 @@ test("prefers a running Bestie instance", () => {
status: "running",
});
assert.equal(pickBestieAgent([stopped, running]), running);
assert.equal(
pickBestieAgent([stopped, running], "wss://buzz.example"),
running,
);
});
test("uses backend-compatible relay identity equivalences", () => {
const loopback = agent({
relayUrl: "WSS://localhost:443/",
personaId: "builtin:bestie",
});
assert.equal(pickBestieAgent([loopback], "wss://[::1]"), loopback);
});
test("fails closed without a valid inherited relay", () => {
const bestie = agent({ personaId: "builtin:bestie" });
assert.equal(pickBestieAgent([bestie], null), null);
assert.equal(pickBestieAgent([bestie], " "), null);
assert.equal(pickBestieAgent([bestie], "not a relay"), null);
assert.equal(pickBestieAgent([bestie], "wss://user@buzz.example"), null);
assert.equal(pickBestieAgent([bestie], "wss://buzz.example/#fragment"), null);
});
test("never selects a Bestie from another relay", () => {
const relayA = agent({
relayUrl: "wss://a.example",
personaId: "builtin:bestie",
});
assert.equal(pickBestieAgent([relayA], "wss://b.example"), null);
});

View File

@@ -1,13 +1,10 @@
import type { ManagedAgent } from "@/shared/api/types";
import { canonicalRelayUrl } from "../managedAgentRuntimeStatus.ts";
export const BESTIE_PERSONA_ID = "builtin:bestie";
const BESTIE_FALLBACK_NAMES = new Set(["bestie", "chief of staff"]);
function normalizeRelayUrl(relayUrl: string | null | undefined) {
return relayUrl?.trim().replace(/\/+$/, "").toLowerCase() ?? null;
}
function preferredByLifecycle(agents: readonly ManagedAgent[]) {
return (
agents.find((agent) => agent.status === "running") ??
@@ -22,11 +19,10 @@ export function pickBestieAgent(
agents: readonly ManagedAgent[],
relayUrl?: string | null,
) {
const normalizedRelayUrl = normalizeRelayUrl(relayUrl);
const normalizedRelayUrl = canonicalRelayUrl(relayUrl ?? "");
if (normalizedRelayUrl === null) return null;
const scoped = agents.filter(
(agent) =>
!normalizedRelayUrl ||
normalizeRelayUrl(agent.relayUrl) === normalizedRelayUrl,
(agent) => canonicalRelayUrl(agent.relayUrl) === normalizedRelayUrl,
);
const builtIn = scoped.filter(
(agent) => agent.personaId === BESTIE_PERSONA_ID,

View File

@@ -96,6 +96,8 @@ test("canonicalRelayUrl mirrors the backend pair-key normalization", () => {
assert.equal(canonicalRelayUrl("ws://[::1]:3000"), "ws://127.0.0.1:3000");
assert.equal(canonicalRelayUrl("https://relay.example"), null);
assert.equal(canonicalRelayUrl("not a url"), null);
assert.equal(canonicalRelayUrl("wss://user@relay.example"), null);
assert.equal(canonicalRelayUrl("wss://relay.example/#fragment"), null);
});
test("matches a stored community URL against canonical backend rows", () => {

View File

@@ -78,6 +78,10 @@ export function canonicalRelayUrl(raw: string): string | null {
return null;
}
if (url.protocol !== "ws:" && url.protocol !== "wss:") return null;
if (url.username !== "" || url.password !== "" || url.hash !== "") {
return null;
}
if (url.hostname === "") return null;
let host = url.hostname.toLowerCase();
if (host === "localhost" || host === "[::1]" || host.startsWith("127.")) {
host = "127.0.0.1";