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.
This commit is contained in:
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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<null | string>(appearance.color)
|
||||
const [image, setImage] = useState<null | string>(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
|
||||
})
|
||||
|
||||
41
apps/desktop/src/plugins/hermes-bots/edit-profile-look.ts
Normal file
41
apps/desktop/src/plugins/hermes-bots/edit-profile-look.ts
Normal file
@@ -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
|
||||
}
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user