From 41b742e338c50d29739eb44a912ec7302135c233 Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Mon, 27 Jul 2026 20:53:19 -0400 Subject: [PATCH 1/3] fix(desktop): use shared create-profile dialog on Manage Profiles page The Manage Profiles page had its own local CreateProfileDialog/ RenameProfileDialog copies that predated the shared dialogs in create-profile-dialog.tsx / rename-profile-dialog.tsx. The local create copy lacked the SOUL.md textarea, so New Profile from the sidebar rail and New Profile from Manage Profiles rendered different modals. Delete both local duplicates and reuse the shared self-contained dialogs (they own the createProfile/renameProfile/updateProfileSoul calls), so both entry points show the same modal including SOUL.md. --- apps/desktop/src/app/profiles/index.tsx | 302 +----------------------- 1 file changed, 12 insertions(+), 290 deletions(-) diff --git a/apps/desktop/src/app/profiles/index.tsx b/apps/desktop/src/app/profiles/index.tsx index b04d9c6c32b9..0a0122764550 100644 --- a/apps/desktop/src/app/profiles/index.tsx +++ b/apps/desktop/src/app/profiles/index.tsx @@ -14,22 +14,11 @@ import { 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 { deleteProfile, 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 +38,8 @@ 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 { RenameProfileDialog } from './rename-profile-dialog' interface ProfilesViewProps { onClose: () => void @@ -112,40 +98,15 @@ 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) + // 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] - ) - - 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) - await refresh() - }, - [p, refresh] + [refresh] ) const handleConfirmDelete = useCallback(async () => { @@ -233,18 +194,13 @@ 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 ?? []} /> @@ -474,237 +430,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} -
- )} - - - - - -
- - - ) -} From 4430a74483fcbfbd52d7601cbe6b8846e4df3df6 Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Tue, 28 Jul 2026 08:33:26 -0400 Subject: [PATCH 2/3] fix(desktop): fold delete dialog into shared, level up name field, test the view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../app/profiles/create-profile-dialog.tsx | 8 +- apps/desktop/src/app/profiles/index.test.tsx | 128 ++++++++++++++++++ apps/desktop/src/app/profiles/index.tsx | 67 ++------- .../app/profiles/rename-profile-dialog.tsx | 8 +- 4 files changed, 149 insertions(+), 62 deletions(-) create mode 100644 apps/desktop/src/app/profiles/index.test.tsx 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..9a10452e9105 --- /dev/null +++ b/apps/desktop/src/app/profiles/index.test.tsx @@ -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('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() +})) + +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() + + 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() + 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() + 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() + }) +}) diff --git a/apps/desktop/src/app/profiles/index.tsx b/apps/desktop/src/app/profiles/index.tsx index 0a0122764550..3f9bad3c4259 100644 --- a/apps/desktop/src/app/profiles/index.tsx +++ b/apps/desktop/src/app/profiles/index.tsx @@ -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) const [pendingDelete, setPendingDelete] = useState(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 ( {!profiles ? ( @@ -205,32 +177,15 @@ export function ProfilesView({ onClose }: ProfilesViewProps) { 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} + /> ) } 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} From a14e25a6822c69a8990f5f9630c6248d79b85ba7 Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Thu, 30 Jul 2026 09:50:28 -0400 Subject: [PATCH 3/3] test(desktop): query the profile row kebab by its own label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main now labels each panel row's kebab with the row's name (menuLabel={profile.name}), so the hardcoded "Actions" default this test relied on no longer exists. The name alone is ambiguous — the row-select button carries it too — so match the menu trigger via `expanded`. Neither side conflicts textually, so this only surfaced once main merged in. --- apps/desktop/src/app/profiles/index.test.tsx | 51 +++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/app/profiles/index.test.tsx b/apps/desktop/src/app/profiles/index.test.tsx index 9a10452e9105..6050c935385d 100644 --- a/apps/desktop/src/app/profiles/index.test.tsx +++ b/apps/desktop/src/app/profiles/index.test.tsx @@ -1,4 +1,4 @@ -import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +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' @@ -57,6 +57,10 @@ vi.mock('@/store/profile', () => ({ 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, @@ -77,18 +81,41 @@ function realClick(el: HTMLElement) { fireEvent.click(el) } -// Open the (only non-default) row's actions menu → Delete → confirm. +// 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 screen.findByRole('button', { name: 'Actions' })) + realClick(await findRowMenu(NAMED_PROFILE)) fireEvent.click(await screen.findByRole('menuitem', { name: /delete/i })) - fireEvent.click(await screen.findByRole('button', { name: 'Delete' })) + 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([]) - render() + await renderProfilesView() realClick(await screen.findByRole('button', { name: 'New profile' })) @@ -99,13 +126,13 @@ describe('ProfilesView', () => { }) it('re-homes to default when the active profile is deleted', async () => { - vi.mocked(refreshProfiles).mockResolvedValue([makeProfile('default', true), makeProfile('work')]) - activeGateway.set('work') + vi.mocked(refreshProfiles).mockResolvedValue([makeProfile('default', true), makeProfile(NAMED_PROFILE)]) + activeGateway.set(NAMED_PROFILE) - render() + await renderProfilesView() await deleteTheNamedProfile() - await waitFor(() => expect(deleteProfile).toHaveBeenCalledWith('work')) + await waitFor(() => expect(deleteProfile).toHaveBeenCalledWith(NAMED_PROFILE)) await waitFor(() => expect(selectProfile).toHaveBeenCalledWith('default')) expect(setActiveProfile).toHaveBeenCalledWith('default') }) @@ -113,13 +140,13 @@ describe('ProfilesView', () => { 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')]) + vi.mocked(refreshProfiles).mockResolvedValue([makeProfile('default', true), makeProfile(NAMED_PROFILE)]) activeGateway.set('default') - render() + await renderProfilesView() await deleteTheNamedProfile() - await waitFor(() => expect(deleteProfile).toHaveBeenCalledWith('work')) + 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()