mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): fold delete dialog into shared, level up name field, test the view
Addresses review on #73013.
1. Manage Profiles used a hand-rolled delete Dialog next to the shared
DeleteProfileDialog in the same folder. That copy missed the active-
profile re-home fix (f764b0400): deleting the profile the gateway is
on stranded it on a dead backend. Switch to the shared dialog, which
owns the deleteProfile call and re-homes to default. Drops
handleConfirmDelete, the deleting state, and the now-unused Dialog*
imports.
2. The name field regressed to a plain Input during the create-dialog
dedup, losing live slugging. Level both shared dialogs up to
SanitizedInput sanitize={slug} so every entry point gets the behavior
Manage Profiles had — the sanitize primitive means callers never
validate-then-reject.
3. Nothing rendered ProfilesView, which is how the drift got in. Add a
behavior test: create dialog exposes SOUL.md, deleting the active
profile re-homes to default, deleting a non-active one does not.
This commit is contained in:
parent
4d9b7718d9
commit
95571de9d7
4 changed files with 149 additions and 62 deletions
|
|
@ -11,12 +11,13 @@ import {
|
|||
DialogTitle
|
||||
} from '@/components/ui/dialog'
|
||||
import { Field, FieldHint } from '@/components/ui/field'
|
||||
import { Input } from '@/components/ui/input'
|
||||
import { SanitizedInput } from '@/components/ui/sanitized-input'
|
||||
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'
|
||||
import { Textarea } from '@/components/ui/textarea'
|
||||
import { createProfile, updateProfileSoul } from '@/hermes'
|
||||
import { useI18n } from '@/i18n'
|
||||
import { AlertTriangle } from '@/lib/icons'
|
||||
import { slug } from '@/lib/sanitize'
|
||||
import type { ProfileInfo } from '@/types/hermes'
|
||||
|
||||
const PROFILE_NAME_RE = /^[a-z0-9][a-z0-9_-]{0,63}$/
|
||||
|
|
@ -101,12 +102,13 @@ export function CreateProfileDialog({
|
|||
|
||||
<form className="grid gap-4" onSubmit={handleSubmit}>
|
||||
<Field htmlFor="new-profile-name" label={p.nameLabel}>
|
||||
<Input
|
||||
<SanitizedInput
|
||||
aria-invalid={invalid}
|
||||
autoFocus
|
||||
id="new-profile-name"
|
||||
onChange={event => setName(event.target.value)}
|
||||
onValueChange={setName}
|
||||
placeholder="my-profile"
|
||||
sanitize={slug}
|
||||
value={name}
|
||||
/>
|
||||
<FieldHint error={invalid}>{p.nameHint}</FieldHint>
|
||||
|
|
|
|||
128
apps/desktop/src/app/profiles/index.test.tsx
Normal file
128
apps/desktop/src/app/profiles/index.test.tsx
Normal file
|
|
@ -0,0 +1,128 @@
|
|||
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import type * as Nanostores from 'nanostores'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { deleteProfile } from '@/hermes'
|
||||
import { refreshProfiles, selectProfile, setActiveProfile } from '@/store/profile'
|
||||
import type { ProfileInfo } from '@/types/hermes'
|
||||
|
||||
import { ProfilesView } from './index'
|
||||
|
||||
// These tests pin the invariant this whole area exists to hold: the Manage
|
||||
// Profiles page and the sidebar rail share ONE set of profile dialogs, so both
|
||||
// "New Profile" entry points render the same modal (SOUL.md included), and
|
||||
// deleting the profile the gateway is on re-homes to default instead of
|
||||
// stranding it on a dead backend. The drift that motivated the fix got in
|
||||
// precisely because nothing rendered this view.
|
||||
|
||||
afterEach(cleanup)
|
||||
|
||||
// Real i18n (useI18n falls back to English with no provider), so labels are the
|
||||
// actual strings — no brittle key snapshot to maintain here.
|
||||
|
||||
// CodeEditor is CodeMirror; the detail pane's SOUL editor doesn't matter to
|
||||
// these behaviors, so stub it out of the jsdom render.
|
||||
vi.mock('@/components/chat/code-editor', () => ({
|
||||
CodeEditor: () => null
|
||||
}))
|
||||
|
||||
vi.mock('@/hermes', () => ({
|
||||
createProfile: vi.fn(async () => ({ name: 'x', ok: true, path: '/x' })),
|
||||
deleteProfile: vi.fn(async () => ({ ok: true, path: '/x' })),
|
||||
getProfileSoul: vi.fn(async () => ({ content: '', exists: true })),
|
||||
renameProfile: vi.fn(async () => ({ name: 'x', ok: true, path: '/x' })),
|
||||
updateProfileSoul: vi.fn(async () => ({ ok: true }))
|
||||
}))
|
||||
|
||||
vi.mock('@/store/notifications', () => ({
|
||||
notify: vi.fn(),
|
||||
notifyError: vi.fn()
|
||||
}))
|
||||
|
||||
const { $activeGatewayProfile: activeGateway, $profileColors } = vi.hoisted(() => {
|
||||
const { atom } = require('nanostores') as typeof Nanostores
|
||||
|
||||
return {
|
||||
$activeGatewayProfile: atom<string>('default'),
|
||||
$profileColors: atom<Record<string, string>>({})
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/store/profile', () => ({
|
||||
$activeGatewayProfile: activeGateway,
|
||||
$profileColors,
|
||||
normalizeProfileKey: (name: null | string | undefined) => (name ?? '').trim() || 'default',
|
||||
refreshProfiles: vi.fn(async () => [] as ProfileInfo[]),
|
||||
selectProfile: vi.fn(),
|
||||
setActiveProfile: vi.fn()
|
||||
}))
|
||||
|
||||
function makeProfile(name: string, isDefault = false): ProfileInfo {
|
||||
return {
|
||||
has_env: false,
|
||||
is_default: isDefault,
|
||||
model: null,
|
||||
name,
|
||||
path: `/home/user/.hermes/profiles/${name}`,
|
||||
provider: null,
|
||||
skill_count: 0
|
||||
}
|
||||
}
|
||||
|
||||
// Radix's trigger opens on the pointerdown/up pair, not the synthetic click
|
||||
// alone — fire the full sequence a real click produces.
|
||||
function realClick(el: HTMLElement) {
|
||||
fireEvent.pointerDown(el, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.pointerUp(el, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.click(el)
|
||||
}
|
||||
|
||||
// Open the (only non-default) row's actions menu → Delete → confirm.
|
||||
async function deleteTheNamedProfile() {
|
||||
realClick(await screen.findByRole('button', { name: 'Actions' }))
|
||||
fireEvent.click(await screen.findByRole('menuitem', { name: /delete/i }))
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Delete' }))
|
||||
}
|
||||
|
||||
describe('ProfilesView', () => {
|
||||
it('opens the shared create dialog with the SOUL.md field (parity with the rail)', async () => {
|
||||
vi.mocked(refreshProfiles).mockResolvedValue([])
|
||||
|
||||
render(<ProfilesView onClose={vi.fn()} />)
|
||||
|
||||
realClick(await screen.findByRole('button', { name: 'New profile' }))
|
||||
|
||||
const soul = await screen.findByLabelText(/SOUL\.md/i)
|
||||
|
||||
expect(soul.tagName).toBe('TEXTAREA')
|
||||
expect(soul.getAttribute('id')).toBe('new-profile-soul')
|
||||
})
|
||||
|
||||
it('re-homes to default when the active profile is deleted', async () => {
|
||||
vi.mocked(refreshProfiles).mockResolvedValue([makeProfile('default', true), makeProfile('work')])
|
||||
activeGateway.set('work')
|
||||
|
||||
render(<ProfilesView onClose={vi.fn()} />)
|
||||
await deleteTheNamedProfile()
|
||||
|
||||
await waitFor(() => expect(deleteProfile).toHaveBeenCalledWith('work'))
|
||||
await waitFor(() => expect(selectProfile).toHaveBeenCalledWith('default'))
|
||||
expect(setActiveProfile).toHaveBeenCalledWith('default')
|
||||
})
|
||||
|
||||
it('leaves the active profile alone when a different profile is deleted', async () => {
|
||||
vi.mocked(selectProfile).mockClear()
|
||||
vi.mocked(setActiveProfile).mockClear()
|
||||
vi.mocked(refreshProfiles).mockResolvedValue([makeProfile('default', true), makeProfile('work')])
|
||||
activeGateway.set('default')
|
||||
|
||||
render(<ProfilesView onClose={vi.fn()} />)
|
||||
await deleteTheNamedProfile()
|
||||
|
||||
await waitFor(() => expect(deleteProfile).toHaveBeenCalledWith('work'))
|
||||
// The dialog closes once the delete settles; a non-active delete must not re-home.
|
||||
await waitFor(() => expect(screen.queryByRole('button', { name: 'Delete' })).toBeNull())
|
||||
expect(selectProfile).not.toHaveBeenCalled()
|
||||
expect(setActiveProfile).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
@ -6,15 +6,7 @@ import { CodeEditor } from '@/components/chat/code-editor'
|
|||
import { PageLoader } from '@/components/page-loader'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Codicon } from '@/components/ui/codicon'
|
||||
import {
|
||||
Dialog,
|
||||
DialogContent,
|
||||
DialogDescription,
|
||||
DialogFooter,
|
||||
DialogHeader,
|
||||
DialogTitle
|
||||
} from '@/components/ui/dialog'
|
||||
import { deleteProfile, getProfileSoul, type ProfileInfo, updateProfileSoul } from '@/hermes'
|
||||
import { getProfileSoul, type ProfileInfo, updateProfileSoul } from '@/hermes'
|
||||
import { useI18n } from '@/i18n'
|
||||
import { AlertTriangle, Save } from '@/lib/icons'
|
||||
import { profileColorSoft, resolveProfileColor } from '@/lib/profile-color'
|
||||
|
|
@ -39,6 +31,7 @@ import {
|
|||
} from '../overlays/panel'
|
||||
|
||||
import { CreateProfileDialog } from './create-profile-dialog'
|
||||
import { DeleteProfileDialog } from './delete-profile-dialog'
|
||||
import { RenameProfileDialog } from './rename-profile-dialog'
|
||||
|
||||
interface ProfilesViewProps {
|
||||
|
|
@ -54,7 +47,6 @@ export function ProfilesView({ onClose }: ProfilesViewProps) {
|
|||
const [createOpen, setCreateOpen] = useState(false)
|
||||
const [pendingRename, setPendingRename] = useState<null | ProfileInfo>(null)
|
||||
const [pendingDelete, setPendingDelete] = useState<null | ProfileInfo>(null)
|
||||
const [deleting, setDeleting] = useState(false)
|
||||
|
||||
const refresh = useCallback(async () => {
|
||||
try {
|
||||
|
|
@ -109,26 +101,6 @@ export function ProfilesView({ onClose }: ProfilesViewProps) {
|
|||
[refresh]
|
||||
)
|
||||
|
||||
const handleConfirmDelete = useCallback(async () => {
|
||||
if (!pendingDelete) {
|
||||
return
|
||||
}
|
||||
|
||||
setDeleting(true)
|
||||
|
||||
try {
|
||||
await deleteProfile(pendingDelete.name)
|
||||
notify({ kind: 'success', title: p.deleted, message: pendingDelete.name })
|
||||
setPendingDelete(null)
|
||||
setSelectedName(null)
|
||||
await refresh()
|
||||
} catch (err) {
|
||||
notifyError(err, p.failedDelete)
|
||||
} finally {
|
||||
setDeleting(false)
|
||||
}
|
||||
}, [p, pendingDelete, refresh])
|
||||
|
||||
return (
|
||||
<Panel closeLabel={p.close} onClose={onClose}>
|
||||
{!profiles ? (
|
||||
|
|
@ -201,32 +173,15 @@ export function ProfilesView({ onClose }: ProfilesViewProps) {
|
|||
profiles={profiles ?? []}
|
||||
/>
|
||||
|
||||
<Dialog onOpenChange={open => !open && !deleting && setPendingDelete(null)} open={pendingDelete !== null}>
|
||||
<DialogContent className="max-w-md">
|
||||
<DialogHeader>
|
||||
<DialogTitle>{p.deleteTitle}</DialogTitle>
|
||||
<DialogDescription>
|
||||
{pendingDelete ? (
|
||||
<>
|
||||
{p.deleteDescPrefix}
|
||||
<span className="font-medium text-foreground">{pendingDelete.name}</span>
|
||||
{p.deleteDescMid}
|
||||
<span className="font-mono text-xs">{pendingDelete.path}</span>
|
||||
{p.deleteDescSuffix}
|
||||
</>
|
||||
) : null}
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
<DialogFooter>
|
||||
<Button disabled={deleting} onClick={() => setPendingDelete(null)} variant="outline">
|
||||
{t.common.cancel}
|
||||
</Button>
|
||||
<Button disabled={deleting} onClick={() => void handleConfirmDelete()} variant="destructive">
|
||||
{deleting ? p.deleting : t.common.delete}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
<DeleteProfileDialog
|
||||
onClose={() => setPendingDelete(null)}
|
||||
onDeleted={async () => {
|
||||
setSelectedName(null)
|
||||
await refresh()
|
||||
}}
|
||||
open={pendingDelete !== null}
|
||||
profile={pendingDelete}
|
||||
/>
|
||||
</Panel>
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,10 +11,11 @@ import {
|
|||
DialogTitle
|
||||
} from '@/components/ui/dialog'
|
||||
import { Field, FieldHint } from '@/components/ui/field'
|
||||
import { Input } from '@/components/ui/input'
|
||||
import { SanitizedInput } from '@/components/ui/sanitized-input'
|
||||
import { renameProfile } from '@/hermes'
|
||||
import { useI18n } from '@/i18n'
|
||||
import { AlertTriangle } from '@/lib/icons'
|
||||
import { slug } from '@/lib/sanitize'
|
||||
|
||||
import { isValidProfileName } from './create-profile-dialog'
|
||||
|
||||
|
|
@ -95,11 +96,12 @@ export function RenameProfileDialog({
|
|||
|
||||
<form className="grid gap-4" onSubmit={handleSubmit}>
|
||||
<Field htmlFor="rename-profile-name" label={p.newNameLabel}>
|
||||
<Input
|
||||
<SanitizedInput
|
||||
aria-invalid={invalid}
|
||||
autoFocus
|
||||
id="rename-profile-name"
|
||||
onChange={event => setName(event.target.value)}
|
||||
onValueChange={setName}
|
||||
sanitize={slug}
|
||||
value={name}
|
||||
/>
|
||||
<FieldHint error={invalid}>{p.nameHint}</FieldHint>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue