fix(desktop): sync agent relay profile when persona avatar changes#1512
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7089313851
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91b3fdbb66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The useUpdatePersonaMutation already invalidated user-profile queries after a persona avatar change, but channel timelines and member lists use useUsersBatchQuery with a 60s staleTime. Without invalidating users-batch, those surfaces show the stale avatar until the batch query naturally refetches. Addresses the outstanding Codex review comment on PR #1512. Signed-off-by: Kenny Lopez <kenny@block.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Reviewed the full diff against the sibling paths it mirrors. The core fix is correct and well-structured:
- Backend faithfully follows the
update_managed_agenttwo-phase pattern (agent_models.rs:800-984): sync params collected undermanaged_agents_store_lockinsidespawn_blocking, publish after the lock drops — no.awaitunder the guard. ✅ - Persisting
record.avatar_urlbefore publishing keeps startup reconciliation (reconcile_agent_profile) in agreement with the published kind:0, so nothing fights back on next launch. ✅ - The frontend priority flip (persona wins over relay profile) matches the codebase's documented philosophy —
resolve_legacy_avatarincommands/agents.rsalready states the persona avatar is authoritative because the relay is an unreliable source for persona-backed agents. ✅ - The
user-profile/users-batchpredicate invalidation matches the existing pattern infeatures/profile/hooks.ts. ✅
A few notes, in descending order of importance:
1. Clearing the persona avatar leaves a restart-dependent state (worth fixing in this PR)
When the user clears the avatar, trim_optional yields None, so this path sets record.avatar_url = None and publishes a kind:0 with no picture. But avatar_url: None is exactly the legacy pre-PR-921 sentinel: on next startup, reconcile_agent_profile treats the record as un-migrated, backfills the command-default icon, and re-publishes. Net behavior: clear → blank avatar until restart → command icon after restart.
The sibling paths all resolve a fallback instead of persisting None:
update_managed_agent(agent_models.rs~938):record.avatar_url.clone().or_else(|| managed_agent_avatar_url(&effective_command))resolve_created_avatar_url(commands/agents.rs~185): same chain.
Suggest mirroring that here — when the persona avatar is cleared, fall back to managed_agent_avatar_url(&effective_command) for the linked agent records and the published profile, so clear-avatar is deterministic instead of restart-dependent.
2. Awaited sync with no HTTP timeout (fine as a follow-up)
Phase 2 awaits one sync_managed_agent_profile per linked agent, sequentially, and AppState.http_client (app_state.rs:89) sets no request timeout — a hung relay stalls the Save mutation indefinitely, multiplied by the number of linked agents. update_managed_agent has the same exposure, so this is consistent rather than a regression; a client-level timeout (or join_all) would fix both paths at once and is better done as its own change.
3. Cosmetic
- The error log here reuses
sync_managed_agent_profile's baked-in "Created the agent, but could not sync its profile metadata" message, which reads oddly on the update path (the rename path shares this wart). - The PR body still says "fire-and-forget" for phase 2, but the second commit correctly changed it to awaited — worth a one-line body edit so the description matches the code.
Only #1 changes user-visible behavior; #2 and #3 are follow-up/cosmetic.
…s cleared When a persona avatar is cleared, trim_optional yields None. Previously this None was propagated directly to linked agent records, which reconcile_agent_profile interprets as "un-migrated legacy record" and backfills with the command-default icon on next restart — making the clear-avatar flow non-deterministic (blank until restart, then icon). Apply the same or_else(managed_agent_avatar_url) fallback already used in update_managed_agent so the agent record always stores a concrete URL. Addresses review feedback from wesbillman on PR #1512.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 096a7ac395
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Thanks for the thorough review @wesbillman! #1 (clear-avatar fallback): Fixed in 096a7ac — when the persona avatar is cleared, linked agent records now fall back to #2 (HTTP timeout): Agreed this is pre-existing and consistent — will address in a follow-up with a client-level timeout or #3 (cosmetic): Will fix the error log wording and PR body description in a follow-up. |
When editing a persona's avatar, the linked agents' relay profiles were never re-synced — the kind:0 event still showed the old picture URL. Backend: After saving the persona, if the avatar changed, update linked agent records' avatar_url and fire-and-forget re-publish their kind:0 profiles to the relay. Frontend: Flip avatar priority in the agent card to prefer the persona avatar (local source of truth) over the relay profile avatar (which may be stale after an edit). Also invalidate user-profile queries on persona update so the relay cache catches up. Fixes: avatar not updating when changing agent profile image
The fire-and-forget spawn allowed the command to return before the relay profile was updated, causing the frontend cache invalidation to refetch stale data. Await the sync inline (matching update_managed_agent's pattern) so the mutation settlement sees the fresh relay profile.
The useUpdatePersonaMutation already invalidated user-profile queries after a persona avatar change, but channel timelines and member lists use useUsersBatchQuery with a 60s staleTime. Without invalidating users-batch, those surfaces show the stale avatar until the batch query naturally refetches. Addresses the outstanding Codex review comment on PR #1512. Signed-off-by: Kenny Lopez <kenny@block.xyz>
…s cleared When a persona avatar is cleared, trim_optional yields None. Previously this None was propagated directly to linked agent records, which reconcile_agent_profile interprets as "un-migrated legacy record" and backfills with the command-default icon on next restart — making the clear-avatar flow non-deterministic (blank until restart, then icon). Apply the same or_else(managed_agent_avatar_url) fallback already used in update_managed_agent so the agent record always stores a concrete URL. Addresses review feedback from wesbillman on PR #1512.
096a7ac to
b77e3fe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b77e3fe62e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sona update When a persona avatar changes, the per-pubkey users-batch-entry cache entries for linked agents must be evicted before invalidating the aggregate users-batch queries. Otherwise the batch refetch resolves from stale 60s-fresh entries, leaving message timelines and member lists showing the old avatar. Mirrors the eviction pattern in useUpdateManagedAgentMutation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c588102fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Renovate's lockfile regen for chrono 0.4.45 (#1050) also downgraded windows-core 0.62.2 -> 0.61.2 (and windows-link/windows-result) for iana-time-zone, wmi, and generator. wmi 0.18.4 uses windows 0.62.2, which cannot compile against windows-core 0.61.2, breaking the Windows Rust job on main. Chrono stays at 0.4.45. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Problem
When editing a persona's avatar in the "Edit agent" dialog, the avatar change was saved to the persona record but the linked agent's relay profile (kind:0 event) was never re-synced. The agent card in the UI continued showing the old avatar because:
update_personasaved the persona locally but never triggeredsync_managed_agent_profilefor linked agents, and never updated the agent record'savatar_urlfield.profileQuery.data?.avatarUrl) over the persona avatar (persona.avatarUrl), so the stale relay data won.Fix
Backend (
desktop/src-tauri/src/commands/personas/mod.rs)avatar_urlfield (with fallback to command-default icon when cleared, matchingupdate_managed_agent)update_managed_agent(sync params collected under lock, async publish outside)Frontend (
desktop/src/features/agents/ui/UnifiedAgentsSection.tsx)firstAvatarUrl(persona.avatarUrl, profileQuery.data?.avatarUrl)Frontend (
desktop/src/features/agents/hooks.ts)user-profileandusers-batchqueries on persona update so the relay profile cache catches up after the backend syncTesting
cargo checkpasses for the Tauri crate