From 7229edf52aabb45d528952e34698eff7240be844 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:23:01 -0700 Subject: [PATCH] fix(desktop): Edit Profile saves only edited fields; stale rows never reconcile Review findings on the bot-identity fix: - Edit Profile sent every seeded field on save, so a color change reverted a rename made elsewhere while the dialog was open (a thin row with no local record even cleared the backend title). The dialog now diffs against the values it opened with (edit-profile-look.ts) and sends only what the user changed. - Rows retained from an earlier paint (source unreachable or connect-on-demand) were reconciled as fresh backend truth, reverting a newer local title during an outage. They are now tagged `retained` and mergeServerMeta skips them and any unreachable-source row. - A title-only answer minted an empty direct record that shadowed a configured alias's appearance in botRosterMeta. A title now only corrects an existing record; the row already carries it. --- .../plugins/hermes-bots/data.bot-meta.test.ts | 33 ++++++++++++++- apps/desktop/src/plugins/hermes-bots/data.ts | 3 +- .../hermes-bots/edit-profile-dialog.tsx | 32 +++++++++------ .../plugins/hermes-bots/edit-profile-look.ts | 41 +++++++++++++++++++ .../src/plugins/hermes-bots/profile-ops.ts | 13 ++++++ apps/desktop/src/plugins/hermes-bots/types.ts | 3 ++ 6 files changed, 111 insertions(+), 14 deletions(-) create mode 100644 apps/desktop/src/plugins/hermes-bots/edit-profile-look.ts diff --git a/apps/desktop/src/plugins/hermes-bots/data.bot-meta.test.ts b/apps/desktop/src/plugins/hermes-bots/data.bot-meta.test.ts index efba5c3431..53004f7131 100644 --- a/apps/desktop/src/plugins/hermes-bots/data.bot-meta.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/data.bot-meta.test.ts @@ -18,7 +18,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { $botMeta, $lastRoster, botMentionTag, botMetaWriteAt, saveBotMeta } from './data' +import { $botMeta, $lastRoster, botMentionTag, botMetaWriteAt, mergeMultiSourceRoster, saveBotMeta } from './data' +import { editedLook } from './edit-profile-look' import { groupSpeakerLabel } from './group-chat' import { buildGroupChatTurnPrompt } from './group-round-prompt' import { displayName } from './labels' @@ -441,4 +442,34 @@ describe('a bot is named by its own backend, never by another bot\'s cached reco $lastRoster.set([]) expect(groupSpeakerLabel('vps::default')).not.toBe('Agent B') }) + + it('only a fresh answer from the bot\'s backend corrects its record, and a title alone never mints one', () => { + $botMeta.set({ 'vps::default': { color: '#00ff00', title: 'New title' } }) + // The VPS stops answering; the pane keeps its last-painted row, which predates the rename. + const painted = { ...vps, title: 'Old title' } as RosterRow + + const kept = mergeMultiSourceRoster( + { profiles: [] }, + { agents: [], sources: [{ connectionId: 'vps', reachable: false }] }, + 'local', + [painted] + ) + + mergeServerMeta(kept.profiles as RosterRow[], Date.now()) + expect($botMeta.get()['vps::default']).toEqual({ color: '#00ff00', title: 'New title' }) + + // No record yet (a configured alias may own this bot's look): the backend's + // title rides the row, and no empty record appears to shadow the alias. + $botMeta.set({}) + mergeServerMeta([{ ...vps, title: 'Agent A' } as RosterRow]) + expect($botMeta.get()).toEqual({}) + }) + + it('Edit Profile sends only what the user changed since it opened', () => { + const opened = { color: '#ff0000', image: null, shape: 'circle' as const, title: 'Old title' } + + // Renamed elsewhere meanwhile; this dialog only changes the color. + expect(editedLook(opened, { ...opened, color: '#0000ff' })).toEqual({ color: '#0000ff', custom: true }) + expect(editedLook(opened, { ...opened, title: ' Old title ' })).toBeNull() + }) }) diff --git a/apps/desktop/src/plugins/hermes-bots/data.ts b/apps/desktop/src/plugins/hermes-bots/data.ts index c9df5a4e1a..ddace56378 100644 --- a/apps/desktop/src/plugins/hermes-bots/data.ts +++ b/apps/desktop/src/plugins/hermes-bots/data.ts @@ -942,7 +942,7 @@ export function cachedUnionRoster(): RosterSnapshot | null { * source label so BotRow can badge them, warm the captured agent, and route * every open directly through that descriptor. Pure — exercised directly by * the tests. */ -function mergeMultiSourceRoster( +export function mergeMultiSourceRoster( local: RosterSnapshot | null | undefined, union: UnionRoster | null | undefined, activeConnectionId?: null | string, @@ -1120,6 +1120,7 @@ function mergeMultiSourceRoster( profiles.push({ ...row, remoteSource: true, + retained: true, sourceScoped: true }) present.add(key) diff --git a/apps/desktop/src/plugins/hermes-bots/edit-profile-dialog.tsx b/apps/desktop/src/plugins/hermes-bots/edit-profile-dialog.tsx index 02315d0dce..a31fc2de34 100644 --- a/apps/desktop/src/plugins/hermes-bots/edit-profile-dialog.tsx +++ b/apps/desktop/src/plugins/hermes-bots/edit-profile-dialog.tsx @@ -25,6 +25,7 @@ import { avatarColor, botAppearance, BotFace } from './avatar' import { AvatarPicker } from './avatar-picker' import { $botMeta, botSelectionKey, ROSTER_KEY, saveBotMeta } from './data' import { labeled } from './dialog-parts' +import { editedLook, type EditProfileLook } from './edit-profile-look' import { useBots } from './i18n' import { displayName } from './labels' import { AdvancedProfileConfig, applyAdvancedConfig, emptyAdvancedState } from './profile-config' @@ -65,7 +66,19 @@ export function EditProfileDialog({ bot, open, onClose }: EditProfileDialogProps const [shape, setShape] = useState(appearance.shape) const [color, setColor] = useState(appearance.color) const [image, setImage] = useState(appearance.image ?? null) - const [title, setTitle] = useState(meta?.title || '') + // A thin row from another connection carries its backend's title on the + // row itself; there may be no local record for it. + const openingTitle = meta?.title || bot?.title || '' + + const opening: EditProfileLook = { + shape: appearance.shape, + color: appearance.color, + image: appearance.image ?? null, + title: openingTitle + } + + const [opened, setOpened] = useState(opening) + const [title, setTitle] = useState(openingTitle) const [description, setDescription] = useState(bot?.description || '') const [busy, setBusy] = useState(false) const [advanced, setAdvanced] = useState(false) @@ -82,7 +95,8 @@ export function EditProfileDialog({ bot, open, onClose }: EditProfileDialogProps setShape(appearance.shape) setColor(appearance.color) setImage(appearance.image ?? null) - setTitle(meta?.title || '') + setTitle(openingTitle) + setOpened(opening) setDescription(bot.description || '') setBusy(false) setAdvanced(false) @@ -102,19 +116,13 @@ export function EditProfileDialog({ bot, open, onClose }: EditProfileDialogProps setBusy(true) let advancedFailed = false - const persistence = await saveBotMeta(bot, { - shape, - color: color ?? undefined, - image, - imageKind: image ? 'photo' : 'shape', - title: title.trim(), - custom: true - }) + const look = editedLook(opened, { shape, color, image, title }) + const persistence = look ? await saveBotMeta(bot, look) : null // Only an explicit remote failure is an error — 'unsupported' is the // documented older-gateway fallback (local wins, silently), and toasting // it would flag every save on every legacy setup forever. - const lookFailed = persistence.serverOutcome === 'failed' + const lookFailed = persistence?.serverOutcome === 'failed' if (lookFailed) { host.notify({ @@ -123,7 +131,7 @@ export function EditProfileDialog({ bot, open, onClose }: EditProfileDialogProps }) } - if (persistence.serverOutcome === 'persisted') { + if (persistence?.serverOutcome === 'persisted') { queryClient.invalidateQueries({ queryKey: ROSTER_KEY }) diff --git a/apps/desktop/src/plugins/hermes-bots/edit-profile-look.ts b/apps/desktop/src/plugins/hermes-bots/edit-profile-look.ts new file mode 100644 index 0000000000..e5c8d723d0 --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/edit-profile-look.ts @@ -0,0 +1,41 @@ +import type { BotMeta } from './types' + +/** What Edit Profile shows for a bot's look, as opened and as submitted. */ +export interface EditProfileLook { + color: null | string + image: null | string + shape: BotMeta['shape'] + title: string +} + +/** The fields the user actually changed since the dialog opened, as a + * saveBotMeta patch; null when nothing changed. Untouched fields stay out of + * the patch: a form value seeded minutes ago is not an edit, and sending it + * would revert whatever another Desktop or the backend wrote since (a rename + * undone by a color change). */ +export function editedLook(opened: EditProfileLook, current: EditProfileLook): BotMeta | null { + const patch: BotMeta = {} + + if (current.shape !== opened.shape) { + patch.shape = current.shape + } + + if (current.color !== opened.color) { + patch.color = current.color ?? undefined + } + + if (current.image !== opened.image) { + patch.image = current.image + patch.imageKind = current.image ? 'photo' : 'shape' + } + + if (Object.keys(patch).length > 0) { + patch.custom = true + } + + if (current.title.trim() !== opened.title.trim()) { + patch.title = current.title.trim() + } + + return Object.keys(patch).length > 0 ? patch : null +} diff --git a/apps/desktop/src/plugins/hermes-bots/profile-ops.ts b/apps/desktop/src/plugins/hermes-bots/profile-ops.ts index 8fe8b51de3..3f48e4ae1b 100644 --- a/apps/desktop/src/plugins/hermes-bots/profile-ops.ts +++ b/apps/desktop/src/plugins/hermes-bots/profile-ops.ts @@ -256,6 +256,12 @@ export function mergeServerMeta(roster: RosterRow[], fetchedAt = 0) { } for (const bot of roster) { + // A row retained from an earlier paint (its source did not answer this + // fetch) is as old as that paint: it cannot correct anything. + if (bot.retained || bot.sourceReachable === false) { + continue + } + const raw = bot.ui_meta?.['hermes-bots'] const server = raw && typeof raw === 'object' ? raw : null const title = serverReportedTitle(bot, server) @@ -267,6 +273,13 @@ export function mergeServerMeta(roster: RosterRow[], fetchedAt = 0) { continue } + // A title alone only corrects an existing record. Minting one would + // shadow a configured alias's appearance (botRosterMeta prefers the + // direct key), and the row already carries its backend's title. + if (!server && !next[key]) { + continue + } + const mine = next[key] || {} const merged = { diff --git a/apps/desktop/src/plugins/hermes-bots/types.ts b/apps/desktop/src/plugins/hermes-bots/types.ts index 14c9f6884b..860445748c 100644 --- a/apps/desktop/src/plugins/hermes-bots/types.ts +++ b/apps/desktop/src/plugins/hermes-bots/types.ts @@ -111,6 +111,9 @@ export interface RosterRow { installId?: string last_session?: SessionPreview | null remoteSource?: boolean + /** Kept from an earlier paint because its source did not answer this + * fetch: shown, but never evidence of what the backend holds now. */ + retained?: boolean route?: ProfileRoute sourceError?: null | string sourceMissing?: boolean