Skip to content

Commit f2f3db7

Browse files
committed
refactor(agents): supersede #1202 avatar-clear with derive-once
Restack derive-once onto the #1202 avatar-plumbing branch and subtract #1202's avatar-clear mechanism. The avatar is a command-derived default icon, so it is derived once at create (resolve_created_avatar_url) and stored as a plain Option<String>; there is no reconcile-time backfill to guard against. Removed: - avatar_url_cleared: bool field and all initializer/test sites - profile_sync_avatar_url .or_else(managed_agent_avatar_url) re-derive - dead Fizz-retirement / legacy-recovery / command-fallback helpers - now-unused app: &AppHandle param on reconcile_agent_profile Leaves #1202's non-avatar plumbing intact (persona/team model fields, app-avatar: prefix parsing, imported app-avatar handling). All three icon-resurrection vectors are now gone. Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
1 parent ba6a90d commit f2f3db7

10 files changed

Lines changed: 22 additions & 383 deletions

File tree

desktop/scripts/check-file-sizes.mjs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const rules = [
3030
// Do not add to this list; split the file instead. Remove each entry as its
3131
// file is broken up. Tracked as a follow-up.
3232
const overrides = new Map([
33-
["src-tauri/src/commands/agents.rs", 1294],
33+
["src-tauri/src/commands/agents.rs", 1110],
3434
// Residual repos_dir integration in ensure_nest_at: REPOS is provisioned
3535
// outside NEST_DIRS (it may be a symlink), so it needs its own create +
3636
// chmod-only-when-real-dir handling plus integration test coverage. The
@@ -41,11 +41,6 @@ const overrides = new Map([
4141
["src-tauri/src/managed_agents/runtime.rs", 1953],
4242
["src-tauri/src/managed_agents/personas.rs", 1080],
4343
["src-tauri/src/managed_agents/persona_card.rs", 1050],
44-
// avatar_url_cleared flag + app-avatar ref plumbing pushed this over the
45-
// 1000-line cap once the branch caught up with main — a small overage from
46-
// load-bearing avatar persistence plumbing, not generic debt growth.
47-
// Approved override; still queued to split with the rest of this list.
48-
["src-tauri/src/managed_agents/types.rs", 1002],
4944
// applyWorkspace reposDir parameter threaded through the Tauri invoke for
5045
// configurable repos_dir — a 3-line overage from load-bearing parameter
5146
// plumbing, not generic debt growth. Approved override; still queued to split.

desktop/src-tauri/src/commands/agent_models.rs

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ use crate::{
77
app_state::AppState,
88
managed_agents::{
99
build_managed_agent_summary, default_agent_workdir, find_managed_agent_mut,
10-
known_acp_runtime, load_managed_agents, load_personas, managed_agent_avatar_url,
11-
missing_command_message, normalize_agent_args, resolve_command,
12-
resolve_effective_prompt_model_provider, save_managed_agents, sync_managed_agent_processes,
13-
try_regenerate_nest, AgentModelInfo, AgentModelsResponse, ManagedAgentRecord,
14-
UpdateManagedAgentRequest, UpdateManagedAgentResponse,
10+
known_acp_runtime, load_managed_agents, load_personas, missing_command_message,
11+
normalize_agent_args, resolve_command, resolve_effective_prompt_model_provider,
12+
save_managed_agents, sync_managed_agent_processes, try_regenerate_nest, AgentModelInfo,
13+
AgentModelsResponse, UpdateManagedAgentRequest, UpdateManagedAgentResponse,
1514
},
1615
relay::{relay_ws_url_with_override, sync_managed_agent_profile},
1716
util::now_iso,
@@ -23,27 +22,6 @@ fn trim_optional(value: Option<String>) -> Option<String> {
2322
.filter(|s| !s.is_empty())
2423
}
2524

26-
fn is_persona_runtime_avatar(record: &ManagedAgentRecord, avatar_url: &str) -> bool {
27-
record.persona_id.is_some()
28-
&& managed_agent_avatar_url(&record.agent_command)
29-
.as_deref()
30-
.is_some_and(|runtime_avatar_url| runtime_avatar_url == avatar_url.trim())
31-
}
32-
33-
fn profile_sync_avatar_url(record: &ManagedAgentRecord) -> Option<String> {
34-
record
35-
.avatar_url
36-
.clone()
37-
.filter(|avatar_url| !is_persona_runtime_avatar(record, avatar_url))
38-
.or_else(|| {
39-
if record.persona_id.is_none() {
40-
managed_agent_avatar_url(&record.agent_command)
41-
} else {
42-
None
43-
}
44-
})
45-
}
46-
4725
/// Query available models from an agent via `buzz-acp models --json`.
4826
///
4927
/// Spawns a short-lived subprocess (no relay connection needed). The subprocess
@@ -200,10 +178,8 @@ pub async fn update_managed_agent(
200178
}
201179
if let Some(avatar_update) = input.avatar_url {
202180
let normalized = trim_optional(avatar_update);
203-
let avatar_url_cleared = normalized.is_none();
204-
if normalized != record.avatar_url || avatar_url_cleared != record.avatar_url_cleared {
181+
if normalized != record.avatar_url {
205182
record.avatar_url = normalized;
206-
record.avatar_url_cleared = avatar_url_cleared;
207183
avatar_changed = true;
208184
}
209185
}
@@ -288,11 +264,7 @@ pub async fn update_managed_agent(
288264
.map_err(|e| format!("failed to parse agent keys: {e}"))?;
289265
let relay_url = record.relay_url.clone();
290266
let display_name = record.name.clone();
291-
let avatar_url = if avatar_changed || record.avatar_url_cleared {
292-
record.avatar_url.clone()
293-
} else {
294-
profile_sync_avatar_url(record)
295-
};
267+
let avatar_url = record.avatar_url.clone();
296268
let auth_tag = record.auth_tag.clone();
297269
Some((agent_keys, relay_url, display_name, avatar_url, auth_tag))
298270
} else {

desktop/src-tauri/src/commands/agents.rs

Lines changed: 14 additions & 198 deletions
Original file line numberDiff line numberDiff line change
@@ -78,28 +78,6 @@ fn resolve_created_avatar_url(
7878
})
7979
}
8080

81-
fn is_retired_fizz_data_url(persona_id: Option<&str>, avatar_url: &str) -> bool {
82-
persona_id == Some("builtin:fizz") && avatar_url.trim_start().starts_with("data:image/")
83-
}
84-
85-
fn is_command_avatar_for_persona(
86-
persona_id: Option<&str>,
87-
agent_command: &str,
88-
avatar_url: &str,
89-
) -> bool {
90-
persona_id.is_some()
91-
&& managed_agent_avatar_url(agent_command)
92-
.as_deref()
93-
.is_some_and(|command_avatar_url| command_avatar_url == avatar_url.trim())
94-
}
95-
96-
fn filter_retired_fizz_avatar(
97-
persona_id: Option<&str>,
98-
avatar_url: Option<String>,
99-
) -> Option<String> {
100-
avatar_url.filter(|url| !is_retired_fizz_data_url(persona_id, url))
101-
}
102-
10381
#[cfg(feature = "mesh-llm")]
10482
async fn ensure_relay_mesh_for_record(
10583
app: &AppHandle,
@@ -551,7 +529,6 @@ pub async fn create_managed_agent(
551529
auth_tag: auth_tag.clone(),
552530
relay_url: resolved_relay_url.clone(),
553531
avatar_url: resolved_avatar_url.clone(),
554-
avatar_url_cleared: false,
555532
acp_command: input
556533
.acp_command
557534
.as_deref()
@@ -763,22 +740,10 @@ pub(crate) struct ProfileReconcileData {
763740
pub(crate) private_key_nsec: String,
764741
pub(crate) name: String,
765742
pub(crate) relay_url: String,
766-
/// Expected avatar URL for the published profile. `None` can mean either a
767-
/// legacy missing value or an explicit clear; `avatar_url_cleared`
768-
/// disambiguates those cases.
743+
/// Expected avatar URL for the published profile. Derived once at creation
744+
/// and stored verbatim; reconciliation republishes it as-is.
769745
pub(crate) avatar_url: Option<String>,
770-
pub(crate) avatar_url_cleared: bool,
771746
pub(crate) auth_tag: Option<String>,
772-
/// The agent's pubkey (hex). Needed to update the persisted record during
773-
/// avatar backfill migration.
774-
pub(crate) pubkey: String,
775-
/// The agent's command (e.g. "goose"). Used as fallback when no profile
776-
/// exists on the relay during avatar backfill.
777-
pub(crate) agent_command: String,
778-
/// Persona ID if this agent was created from a persona. Used during avatar
779-
/// backfill to recover the correct avatar from the persona record when the
780-
/// relay profile has been corrupted.
781-
pub(crate) persona_id: Option<String>,
782747
}
783748

784749
#[tauri::command]
@@ -823,11 +788,7 @@ pub async fn start_managed_agent(
823788
name: record.name.clone(),
824789
relay_url: record.relay_url.clone(),
825790
avatar_url: record.avatar_url.clone(),
826-
avatar_url_cleared: record.avatar_url_cleared,
827791
auth_tag: record.auth_tag.clone(),
828-
pubkey: record.pubkey.clone(),
829-
agent_command: record.agent_command.clone(),
830-
persona_id: record.persona_id.clone(),
831792
};
832793

833794
let target = if record.backend == BackendKind::Local {
@@ -888,17 +849,16 @@ pub async fn start_managed_agent(
888849
// ── Profile reconciliation (fire-and-forget) ────────────────────────────
889850
// On successful start, spawn a background task to ensure the agent's kind:0
890851
// profile is published on the relay. This self-heals cases where the initial
891-
// profile sync at creation time failed silently. For legacy records (pre-PR-921)
892-
// with no persisted avatar, this also backfills the avatar from the relay.
852+
// profile sync at creation time failed silently. The avatar derived once at
853+
// creation is published verbatim — there is no reconcile-time backfill.
893854
if result.is_ok() {
894855
let reconcile_pubkey = pubkey.clone();
895856
let reconcile_app = app.clone();
896857
tauri::async_runtime::spawn(async move {
897858
use tauri::Manager;
898859
let state = reconcile_app.state::<AppState>();
899860
if let Err(e) =
900-
reconcile_agent_profile(&state, &reconcile_app, &reconcile_pubkey, &reconcile_data)
901-
.await
861+
reconcile_agent_profile(&state, &reconcile_pubkey, &reconcile_data).await
902862
{
903863
eprintln!(
904864
"buzz-desktop: profile reconciliation failed for agent {reconcile_pubkey}: {e}"
@@ -910,59 +870,19 @@ pub async fn start_managed_agent(
910870
result
911871
}
912872

913-
/// Resolve the avatar to backfill for a legacy agent record (pre-PR-921, no
914-
/// stored `avatar_url`).
915-
///
916-
/// Priority: the persona's avatar wins, because the old reconciliation code
917-
/// could have overwritten the relay's kind:0 `picture` with the command default
918-
/// — making the relay an unreliable source for persona-backed agents. Only fall
919-
/// back to the relay's `picture`, then the command icon, for agents with no
920-
/// persona avatar to recover from.
921-
fn resolve_legacy_avatar(
922-
persona_avatar: Option<String>,
923-
relay_picture: Option<String>,
924-
agent_command: &str,
925-
use_command_fallback: bool,
926-
) -> String {
927-
persona_avatar
928-
.or(relay_picture)
929-
.or_else(|| {
930-
if use_command_fallback {
931-
managed_agent_avatar_url(agent_command)
932-
} else {
933-
None
934-
}
935-
})
936-
.unwrap_or_default()
937-
}
938-
939-
fn should_skip_legacy_command_avatar(
940-
stored_avatar_was_retired_fizz: bool,
941-
relay_picture_was_retired_fizz: bool,
942-
persona_avatar: Option<&str>,
943-
relay_picture: Option<&str>,
944-
) -> bool {
945-
(stored_avatar_was_retired_fizz || relay_picture_was_retired_fizz)
946-
&& persona_avatar.is_none()
947-
&& relay_picture.is_none()
948-
}
949-
950873
/// Reconcile an agent's kind:0 profile on the relay.
951874
///
952875
/// Queries the relay for the agent's existing profile and re-publishes if missing
953876
/// or stale. This is fire-and-forget — errors are returned to the caller for
954877
/// logging but never block agent startup.
955878
///
956-
/// For legacy records (pre-PR-921) where `avatar_url` is `None`, this function
957-
/// backfills via `resolve_legacy_avatar` — preferring the persona record's avatar
958-
/// over the relay's `picture`, since the old code may have corrupted the relay
959-
/// profile — and persists the updated record. After backfill, normal
960-
/// Query and publish both target the agent's stored `relay_url` so that, under
961-
/// an active workspace relay override, reconciliation reads and writes the same
962-
/// relay the agent's profile actually lives on.
879+
/// The avatar is derived once at creation and stored on the record; reconcile
880+
/// publishes that stored value verbatim with no backfill. Query and publish both
881+
/// target the agent's stored `relay_url` so that, under an active workspace relay
882+
/// override, reconciliation reads and writes the same relay the agent's profile
883+
/// actually lives on.
963884
pub(crate) async fn reconcile_agent_profile(
964885
state: &AppState,
965-
app: &AppHandle,
966886
agent_pubkey: &str,
967887
data: &ProfileReconcileData,
968888
) -> Result<(), String> {
@@ -971,114 +891,10 @@ pub(crate) async fn reconcile_agent_profile(
971891
// Query the relay for the agent's existing kind:0 profile.
972892
let existing = query_agent_profile(state, &data.relay_url, agent_pubkey).await?;
973893

974-
// Resolve the expected avatar. A user-initiated clear is intentionally
975-
// `None` and must not be backfilled from persona/relay/runtime defaults.
976-
// For legacy records that have no stored avatar_url yet, `None` still means
977-
// backfill from the best available historical source.
978-
let stored_avatar =
979-
filter_retired_fizz_avatar(data.persona_id.as_deref(), data.avatar_url.clone());
980-
let stored_avatar_was_retired_fizz = data
981-
.avatar_url
982-
.as_deref()
983-
.is_some_and(|url| is_retired_fizz_data_url(data.persona_id.as_deref(), url));
984-
let stored_avatar_was_command_fallback = stored_avatar.as_deref().is_some_and(|url| {
985-
is_command_avatar_for_persona(data.persona_id.as_deref(), &data.agent_command, url)
986-
});
987-
let stored_avatar = stored_avatar.filter(|url| {
988-
!is_command_avatar_for_persona(data.persona_id.as_deref(), &data.agent_command, url)
989-
});
990-
let expected_avatar = if data.avatar_url_cleared && stored_avatar.is_none() {
991-
None
992-
} else {
993-
match stored_avatar {
994-
Some(url) => Some(url.to_string()),
995-
None => {
996-
// Legacy record: the relay profile may have been corrupted by the
997-
// old reconciliation code (it overwrote the persona avatar with the
998-
// command default), so the persona record is the authoritative source.
999-
let persona_avatar = filter_retired_fizz_avatar(
1000-
data.persona_id.as_deref(),
1001-
data.persona_id.as_ref().and_then(|pid| {
1002-
load_personas(app)
1003-
.ok()?
1004-
.into_iter()
1005-
.find(|p| p.id == *pid)?
1006-
.avatar_url
1007-
}),
1008-
);
1009-
let relay_picture_raw = existing.as_ref().and_then(|info| info.picture.clone());
1010-
let relay_picture_was_retired_fizz = relay_picture_raw
1011-
.as_deref()
1012-
.is_some_and(|url| is_retired_fizz_data_url(data.persona_id.as_deref(), url));
1013-
let relay_picture =
1014-
filter_retired_fizz_avatar(data.persona_id.as_deref(), relay_picture_raw);
1015-
let relay_picture_was_command_fallback =
1016-
relay_picture.as_deref().is_some_and(|url| {
1017-
is_command_avatar_for_persona(
1018-
data.persona_id.as_deref(),
1019-
&data.agent_command,
1020-
url,
1021-
)
1022-
});
1023-
let relay_picture = relay_picture.filter(|url| {
1024-
!is_command_avatar_for_persona(
1025-
data.persona_id.as_deref(),
1026-
&data.agent_command,
1027-
url,
1028-
)
1029-
});
1030-
1031-
let skip_command_fallback = should_skip_legacy_command_avatar(
1032-
stored_avatar_was_retired_fizz,
1033-
relay_picture_was_retired_fizz,
1034-
persona_avatar.as_deref(),
1035-
relay_picture.as_deref(),
1036-
);
1037-
let backfilled = if skip_command_fallback {
1038-
String::new()
1039-
} else {
1040-
resolve_legacy_avatar(
1041-
persona_avatar,
1042-
relay_picture,
1043-
&data.agent_command,
1044-
data.persona_id.is_none(),
1045-
)
1046-
};
1047-
1048-
// Persist the backfilled avatar so this migration only runs once,
1049-
// or clear the retired built-in Fizz data URL if there is no
1050-
// current profile image to backfill.
1051-
let should_persist_avatar = stored_avatar_was_retired_fizz
1052-
|| relay_picture_was_retired_fizz
1053-
|| stored_avatar_was_command_fallback
1054-
|| relay_picture_was_command_fallback
1055-
|| (!backfilled.is_empty()
1056-
&& data.avatar_url.as_deref() != Some(backfilled.as_str()));
1057-
if should_persist_avatar {
1058-
let _store_guard = state
1059-
.managed_agents_store_lock
1060-
.lock()
1061-
.map_err(|e| e.to_string())?;
1062-
let mut records = load_managed_agents(app)?;
1063-
if let Some(record) = records.iter_mut().find(|r| r.pubkey == data.pubkey) {
1064-
record.avatar_url = if backfilled.is_empty() {
1065-
None
1066-
} else {
1067-
Some(backfilled.clone())
1068-
};
1069-
record.avatar_url_cleared = backfilled.is_empty();
1070-
save_managed_agents(app, &records)?;
1071-
}
1072-
}
1073-
1074-
if backfilled.is_empty() {
1075-
None
1076-
} else {
1077-
Some(backfilled)
1078-
}
1079-
}
1080-
}
1081-
};
894+
// Republish the avatar exactly as derived once at creation. There is no
895+
// reconcile-time backfill: a stored `None` (including an explicit clear)
896+
// is published verbatim.
897+
let expected_avatar = data.avatar_url.clone();
1082898

1083899
if !profile_needs_sync(existing.as_ref(), &data.name, expected_avatar.as_deref()) {
1084900
return Ok(());

0 commit comments

Comments
 (0)