diff --git a/apps/desktop/src/app/profiles/create-profile-dialog.tsx b/apps/desktop/src/app/profiles/create-profile-dialog.tsx index c5bf60ad8c46..67fbc96f38b7 100644 --- a/apps/desktop/src/app/profiles/create-profile-dialog.tsx +++ b/apps/desktop/src/app/profiles/create-profile-dialog.tsx @@ -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({
- setName(event.target.value)} + onValueChange={setName} placeholder="my-profile" + sanitize={slug} value={name} /> {p.nameHint} diff --git a/apps/desktop/src/app/profiles/index.test.tsx b/apps/desktop/src/app/profiles/index.test.tsx new file mode 100644 index 000000000000..6050c935385d --- /dev/null +++ b/apps/desktop/src/app/profiles/index.test.tsx @@ -0,0 +1,155 @@ +import { act, 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('default'), + $profileColors: atom>({}) + } +}) + +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() +})) + +// The one non-default profile these tests act on. Its name doubles as the row's +// accessible name, so the delete helper queries by it rather than a literal. +const NAMED_PROFILE = 'work' + +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) +} + +// ProfilesView loads its list in a mount effect (refreshProfiles → setProfiles), +// so the first paint is the loader and the rows commit a microtask later. Flush +// that inside act() so the rows exist before anything queries them, and so the +// mount setState isn't left unwrapped. +async function renderProfilesView() { + await act(async () => { + render() + }) +} + +// PanelListRow labels BOTH the row's select target and its kebab with the +// profile name (`menuLabel={profile.name}`), so the name alone matches two +// buttons. Only the kebab is a menu trigger, so `expanded` disambiguates. +function findRowMenu(profileName: string) { + return screen.findByRole('button', { expanded: false, name: profileName }) +} + +// Open the (only non-default) row's actions menu → Delete → confirm. The +// confirm click kicks off an async chain (deleteProfile → onDeleted refresh → +// setProfiles, plus the re-home writes), so settle it inside act() to flush +// those updates deterministically instead of leaking them past the assertions. +async function deleteTheNamedProfile() { + realClick(await findRowMenu(NAMED_PROFILE)) + fireEvent.click(await screen.findByRole('menuitem', { name: /delete/i })) + const confirm = await screen.findByRole('button', { name: 'Delete' }) + await act(async () => { + fireEvent.click(confirm) + }) +} + +describe('ProfilesView', () => { + it('opens the shared create dialog with the SOUL.md field (parity with the rail)', async () => { + vi.mocked(refreshProfiles).mockResolvedValue([]) + + await renderProfilesView() + + 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(NAMED_PROFILE)]) + activeGateway.set(NAMED_PROFILE) + + await renderProfilesView() + await deleteTheNamedProfile() + + await waitFor(() => expect(deleteProfile).toHaveBeenCalledWith(NAMED_PROFILE)) + 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(NAMED_PROFILE)]) + activeGateway.set('default') + + await renderProfilesView() + await deleteTheNamedProfile() + + await waitFor(() => expect(deleteProfile).toHaveBeenCalledWith(NAMED_PROFILE)) + // 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() + }) +}) diff --git a/apps/desktop/src/app/profiles/index.tsx b/apps/desktop/src/app/profiles/index.tsx index 6b30a0789f6a..1db67b8778e3 100644 --- a/apps/desktop/src/app/profiles/index.tsx +++ b/apps/desktop/src/app/profiles/index.tsx @@ -6,30 +6,11 @@ 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 { SanitizedInput } from '@/components/ui/sanitized-input' -import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' -import { - createProfile, - deleteProfile, - getProfileSoul, - type ProfileInfo, - renameProfile, - 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' -import { slug } from '@/lib/sanitize' import { normalize } from '@/lib/text' -import { cn } from '@/lib/utils' import { notify, notifyError } from '@/store/notifications' import { $profileColors, refreshProfiles } from '@/store/profile' @@ -49,11 +30,9 @@ import { PanelSectionLabel } from '../overlays/panel' -const PROFILE_NAME_RE = /^[a-z0-9][a-z0-9_-]{0,63}$/ - -function isValidProfileName(name: string): boolean { - return PROFILE_NAME_RE.test(name.trim()) -} +import { CreateProfileDialog } from './create-profile-dialog' +import { DeleteProfileDialog } from './delete-profile-dialog' +import { RenameProfileDialog } from './rename-profile-dialog' interface ProfilesViewProps { onClose: () => void @@ -68,7 +47,6 @@ export function ProfilesView({ onClose }: ProfilesViewProps) { const [createOpen, setCreateOpen] = useState(false) const [pendingRename, setPendingRename] = useState(null) const [pendingDelete, setPendingDelete] = useState(null) - const [deleting, setDeleting] = useState(false) const refresh = useCallback(async () => { try { @@ -112,62 +90,17 @@ export function ProfilesView({ onClose }: ProfilesViewProps) { ) }, [profiles, query]) - const handleCreate = useCallback( - async (name: string, cloneFrom: null | string) => { - const trimmed = name.trim() - - if (!isValidProfileName(trimmed)) { - throw new Error(p.nameHint) - } - - await createProfile({ name: trimmed, clone_from: cloneFrom }) - notify({ kind: 'success', title: p.created, message: trimmed }) - setSelectedName(trimmed) - await refresh() - }, - [p, refresh] - ) - - const handleRename = useCallback( - async (from: string, to: string): Promise => { - const target = to.trim() - - if (target === from) { - return - } - - if (!isValidProfileName(target)) { - throw new Error(p.nameHint) - } - - await renameProfile(from, target) - notify({ kind: 'success', title: p.renamed, message: `${from} → ${target}` }) - setSelectedName(target) + // The shared Create/Rename dialogs own the createProfile / renameProfile / + // updateProfileSoul calls; the panel just selects the resulting profile and + // re-pulls the list. + const selectAndRefresh = useCallback( + async (name: string) => { + setSelectedName(name) await refresh() }, - [p, refresh] + [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 ( {!profiles ? ( @@ -229,48 +162,26 @@ export function ProfilesView({ onClose }: ProfilesViewProps) { setPendingRename(null)} - onRename={async newName => { - if (pendingRename) { - await handleRename(pendingRename.name, newName) - setPendingRename(null) - } - }} + onRenamed={selectAndRefresh} open={pendingRename !== null} /> setCreateOpen(false)} - onCreate={async (name, cloneFrom) => handleCreate(name, cloneFrom)} + onCreated={selectAndRefresh} open={createOpen} profiles={profiles ?? []} /> - !open && !deleting && setPendingDelete(null)} open={pendingDelete !== null}> - - - {p.deleteTitle} - - {pendingDelete ? ( - <> - {p.deleteDescPrefix} - {pendingDelete.name} - {p.deleteDescMid} - {pendingDelete.path} - {p.deleteDescSuffix} - - ) : null} - - - - - - - - + setPendingDelete(null)} + onDeleted={async () => { + setSelectedName(null) + await refresh() + }} + open={pendingDelete !== null} + profile={pendingDelete} + /> ) } @@ -471,237 +382,3 @@ function SoulEditor({ profileName }: { profileName: string }) { ) } - -function CreateProfileDialog({ - onClose, - onCreate, - open, - profiles -}: { - onClose: () => void - onCreate: (name: string, cloneFrom: null | string) => Promise - open: boolean - profiles: ProfileInfo[] -}) { - const { t } = useI18n() - const p = t.profiles - const [name, setName] = useState('') - const [cloneFrom, setCloneFrom] = useState('default') - const [saving, setSaving] = useState(false) - const [error, setError] = useState(null) - - useEffect(() => { - if (!open) { - return - } - - setName('') - setCloneFrom('default') - setError(null) - setSaving(false) - }, [open]) - - const trimmed = name.trim() - const invalid = trimmed !== '' && !isValidProfileName(trimmed) - - async function handleSubmit(event: React.FormEvent) { - event.preventDefault() - - if (!trimmed || invalid) { - setError(invalid ? p.invalidName(p.nameHint) : p.nameRequired) - - return - } - - setSaving(true) - setError(null) - - try { - await onCreate(trimmed, cloneFrom) - onClose() - } catch (err) { - setError(err instanceof Error ? err.message : p.failedCreate) - } finally { - setSaving(false) - } - } - - return ( - !value && !saving && onClose()} open={open}> - - - {p.newProfile} - {p.createDesc} - - - -
- - -

- {p.nameHint} -

-
- -
- - -

{p.cloneFromDesc}

-
- - {error && ( -
- - {error} -
- )} - - - - - - - - - ) -} - -function RenameProfileDialog({ - currentName, - onClose, - onRename, - open -}: { - currentName: string - onClose: () => void - onRename: (newName: string) => Promise - open: boolean -}) { - const { t } = useI18n() - const p = t.profiles - const [name, setName] = useState(currentName) - const [saving, setSaving] = useState(false) - const [error, setError] = useState(null) - - useEffect(() => { - if (!open) { - return - } - - setName(currentName) - setError(null) - setSaving(false) - }, [currentName, open]) - - const trimmed = name.trim() - const unchanged = trimmed === currentName - const invalid = trimmed !== '' && !unchanged && !isValidProfileName(trimmed) - - async function handleSubmit(event: React.FormEvent) { - event.preventDefault() - - if (unchanged) { - onClose() - - return - } - - if (!trimmed || invalid) { - setError(invalid ? p.invalidName(p.nameHint) : p.nameRequired) - - return - } - - setSaving(true) - setError(null) - - try { - await onRename(trimmed) - } catch (err) { - setError(err instanceof Error ? err.message : p.failedRename) - } finally { - setSaving(false) - } - } - - return ( - !value && !saving && onClose()} open={open}> - - - {p.renameTitle} - - {p.renameDescPrefix} - ~/.local/bin - {p.renameDescSuffix} - - - -
-
- - -

- {p.nameHint} -

-
- - {error && ( -
- - {error} -
- )} - - - - - -
- - - ) -} diff --git a/apps/desktop/src/app/profiles/rename-profile-dialog.tsx b/apps/desktop/src/app/profiles/rename-profile-dialog.tsx index 8a12f7b65ab8..7973472260b0 100644 --- a/apps/desktop/src/app/profiles/rename-profile-dialog.tsx +++ b/apps/desktop/src/app/profiles/rename-profile-dialog.tsx @@ -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({
- setName(event.target.value)} + onValueChange={setName} + sanitize={slug} value={name} /> {p.nameHint}