diff --git a/src/components/settings/agents/add-custom-agent-dialog.test.tsx b/src/components/settings/agents/add-custom-agent-dialog.test.tsx index 8140660e8..2fbbd7259 100644 --- a/src/components/settings/agents/add-custom-agent-dialog.test.tsx +++ b/src/components/settings/agents/add-custom-agent-dialog.test.tsx @@ -5,6 +5,7 @@ import '@testing-library/jest-dom' import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, describe, expect, it, mock } from 'bun:test' +import type { Agent } from '@/types/acp' import { AddCustomAgentDialog, inferTransport, @@ -295,3 +296,116 @@ describe('AddCustomAgentDialog — connection status', () => { expect(screen.queryByText(/connection successful/i)).not.toBeInTheDocument() }) }) + +describe('AddCustomAgentDialog — edit mode', () => { + const notIos = () => false + + const existingAgent: Agent = { + id: 'custom-1', + name: 'Existing Agent', + type: 'remote-acp', + transport: 'websocket', + url: 'wss://existing.example/ws', + description: 'Existing description', + icon: null, + isSystem: 0, + enabled: 1, + deletedAt: null, + userId: 'user-42', + } + + it('renders the Edit title and Save Changes button when editingAgent is set', () => { + const onSubmit = mock(async () => {}) + render( + {}} + onSubmit={onSubmit} + editingAgent={existingAgent} + isIos={notIos} + testAcpConnection={async () => ({ success: true })} + />, + ) + + expect(screen.getByText(/edit custom agent/i)).toBeInTheDocument() + expect(screen.getByRole('button', { name: /save changes/i })).toBeInTheDocument() + // Add Agent label must not appear in edit mode. + expect(screen.queryByRole('button', { name: /^add agent$/i })).not.toBeInTheDocument() + }) + + it('seeds the form with the existing agent values', () => { + render( + {}} + onSubmit={async () => {}} + editingAgent={existingAgent} + isIos={notIos} + testAcpConnection={async () => ({ success: true })} + />, + ) + + expect(screen.getByLabelText(/name/i)).toHaveValue('Existing Agent') + expect(screen.getByLabelText(/url/i)).toHaveValue('wss://existing.example/ws') + expect(screen.getByLabelText(/description/i)).toHaveValue('Existing description') + }) + + it('keeps Save Changes gated until the seeded URL is re-tested', async () => { + render( + {}} + onSubmit={async () => {}} + editingAgent={existingAgent} + isIos={notIos} + testAcpConnection={async () => ({ success: true })} + />, + ) + + const save = screen.getByRole('button', { name: /save changes/i }) + // Form is prefilled but connection has not been re-verified yet. + expect(save).toBeDisabled() + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /test connection/i })) + }) + + expect(save).not.toBeDisabled() + }) + + it('invokes onSubmit with the edited values after a successful test', async () => { + const onSubmit = mock(async (_: AddCustomAgentPayload) => {}) + const onOpenChange = mock(() => {}) + render( + ({ success: true })} + />, + ) + + fireEvent.change(screen.getByLabelText(/name/i), { target: { value: 'Renamed Agent' } }) + fireEvent.change(screen.getByLabelText(/url/i), { target: { value: 'wss://new.example/ws' } }) + fireEvent.change(screen.getByLabelText(/description/i), { target: { value: '' } }) + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /test connection/i })) + }) + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /save changes/i })) + }) + + expect(onSubmit).toHaveBeenCalledTimes(1) + expect(onSubmit).toHaveBeenCalledWith({ + name: 'Renamed Agent', + url: 'wss://new.example/ws', + // Empty description is normalized to null, matching the create path. + description: null, + transport: 'websocket', + }) + expect(onOpenChange).toHaveBeenCalledWith(false) + }) +}) diff --git a/src/components/settings/agents/add-custom-agent-dialog.tsx b/src/components/settings/agents/add-custom-agent-dialog.tsx index b3c66bc9c..59c294e53 100644 --- a/src/components/settings/agents/add-custom-agent-dialog.tsx +++ b/src/components/settings/agents/add-custom-agent-dialog.tsx @@ -17,6 +17,7 @@ import { Dialog } from '@/components/ui/dialog' import { StatusCard } from '@/components/ui/status-card' import { getPlatform, isTauri } from '@/lib/platform' import { testAcpConnection as defaultTestAcpConnection } from '@/acp' +import type { Agent } from '@/types/acp' /** Maps a user-entered URL to the ACP transport flavor we support, or `null` * when the scheme is unsupported (or the URL is malformed). WebSocket is the @@ -72,6 +73,11 @@ type AddCustomAgentDialogProps = { open: boolean onOpenChange: (open: boolean) => void onSubmit: (payload: AddCustomAgentPayload) => Promise | void + /** When provided, the dialog renders in edit mode: title and submit label + * switch, and initial state is seeded from this agent. Pass `null`/omit for + * the create flow. The parent should also vary the dialog's React `key` on + * the agent id so switching between agents resets the reducer cleanly. */ + editingAgent?: Agent | null /** Test/DI override for the iOS guard. Production callers omit this. */ isIos?: () => boolean /** Test/DI override for the connection probe. Production callers omit this. */ @@ -97,9 +103,9 @@ type AgentDialogAction = | { type: 'START_CONNECTION_TEST' } | { type: 'CONNECTION_TEST_SUCCESS' } | { type: 'CONNECTION_TEST_FAILURE'; error: string } - | { type: 'RESET' } + | { type: 'RESET'; next: AgentDialogState } -const initialState: AgentDialogState = { +const emptyState: AgentDialogState = { name: '', url: '', description: '', @@ -109,13 +115,26 @@ const initialState: AgentDialogState = { connectionError: null, } +/** Builds the initial reducer state. With an agent, the form is seeded with its + * current values (a connection test is still required before save). Without, + * the form starts blank for the create flow. */ +const buildInitialState = (agent: Agent | null): AgentDialogState => + agent + ? { + ...emptyState, + name: agent.name, + url: agent.url ?? '', + description: agent.description ?? '', + } + : emptyState + const agentDialogReducer = (state: AgentDialogState, action: AgentDialogAction): AgentDialogState => { switch (action.type) { case 'SET_NAME': return { ...state, name: action.value } case 'SET_URL': // Editing the URL invalidates any prior connection result — the user is - // targeting a (potentially) different endpoint, so Add must be re-gated. + // targeting a (potentially) different endpoint, so submit must be re-gated. return { ...state, url: action.value, connectionStatus: 'idle', connectionError: null } case 'SET_DESCRIPTION': return { ...state, description: action.value } @@ -130,7 +149,7 @@ const agentDialogReducer = (state: AgentDialogState, action: AgentDialogAction): case 'CONNECTION_TEST_FAILURE': return { ...state, isTestingConnection: false, connectionStatus: 'error', connectionError: action.error } case 'RESET': - return initialState + return action.next default: return state } @@ -140,20 +159,25 @@ export const AddCustomAgentDialog = ({ open, onOpenChange, onSubmit, + editingAgent, isIos, testAcpConnection = defaultTestAcpConnection, }: AddCustomAgentDialogProps) => { - const [state, dispatch] = useReducer(agentDialogReducer, initialState) + const isEditing = !!editingAgent + // Lazy init seeds the form from the agent on first mount. The parent varies + // the React `key` on agent id to remount when switching between editing + // targets, so this initializer fires fresh each time. + const [state, dispatch] = useReducer(agentDialogReducer, editingAgent ?? null, buildInitialState) const trimmedName = state.name.trim() const trimmedUrl = state.url.trim() const trimmedDescription = state.description.trim() const validation = validateAgentUrl(trimmedUrl, isIos) // Surface an invalid-URL error at render time (once the field is non-empty) - // so the user sees why Test Connection is unavailable and Add stays gated. + // so the user sees why Test Connection is unavailable and submit stays gated. const urlError = trimmedUrl.length > 0 && 'error' in validation ? validation.error : null - // Add is gated behind a successful Test Connection — a valid name, URL, and a - // confirmed connection are all required before the agent can be created. + // Submit is gated behind a successful Test Connection — a valid name, URL, + // and a confirmed connection are all required before saving. const canSubmit = trimmedName.length > 0 && trimmedUrl.length > 0 && state.connectionStatus === 'success' && !state.submitting // The probe is only meaningful once the URL is a valid WebSocket endpoint. @@ -161,7 +185,9 @@ export const AddCustomAgentDialog = ({ const handleOpenChange = (next: boolean) => { if (!next) { - dispatch({ type: 'RESET' }) + // On close, reset back to the seeded state (empty for create, the agent's + // values for edit) so a reopen without remount lands in a predictable shape. + dispatch({ type: 'RESET', next: buildInitialState(editingAgent ?? null) }) } onOpenChange(next) } @@ -190,7 +216,7 @@ export const AddCustomAgentDialog = ({ transport: validation.transport, }) dispatch({ type: 'END_SUBMIT' }) - dispatch({ type: 'RESET' }) + dispatch({ type: 'RESET', next: buildInitialState(editingAgent ?? null) }) onOpenChange(false) } @@ -198,9 +224,11 @@ export const AddCustomAgentDialog = ({ - Add Custom Agent + {isEditing ? 'Edit Custom Agent' : 'Add Custom Agent'} - Connect a remote agent that speaks the Agent Client Protocol. + {isEditing + ? 'Update the connection details for this remote agent.' + : 'Connect a remote agent that speaks the Agent Client Protocol.'}
@@ -290,7 +318,7 @@ export const AddCustomAgentDialog = ({ Cancel
diff --git a/src/components/settings/agents/agent-list.test.tsx b/src/components/settings/agents/agent-list.test.tsx index 5b3541789..5c136bd58 100644 --- a/src/components/settings/agents/agent-list.test.tsx +++ b/src/components/settings/agents/agent-list.test.tsx @@ -8,7 +8,7 @@ import { afterEach, describe, expect, it, mock } from 'bun:test' import { builtInAgent } from '@/defaults/agents' import type { Agent } from '@/types/acp' import { AgentList } from './agent-list' -import { agentToggleDisabled, canDeleteAgent } from './agent-row' +import { agentToggleDisabled, canDeleteAgent, canEditAgent } from './agent-row' afterEach(() => { cleanup() @@ -42,6 +42,8 @@ const customAgent: Agent = { userId: 'user-42', } +const noop = () => {} + describe('canDeleteAgent', () => { it('returns false for the built-in agent', () => { expect(canDeleteAgent(builtInAgent, 'user-42')).toBe(false) @@ -64,17 +66,25 @@ describe('canDeleteAgent', () => { }) }) +describe('canEditAgent', () => { + it('mirrors canDeleteAgent — built-in and system are non-editable, customs are owned by the user', () => { + expect(canEditAgent(builtInAgent, 'user-42')).toBe(false) + expect(canEditAgent(systemAgent, 'user-42')).toBe(false) + expect(canEditAgent(customAgent, 'user-42')).toBe(true) + expect(canEditAgent(customAgent, 'someone-else')).toBe(false) + expect(canEditAgent(customAgent, null)).toBe(false) + }) +}) + describe('AgentList', () => { it('renders rows for built-in, system, and custom agents in the given order', () => { - const onToggle = mock(() => {}) - const onDelete = mock(() => {}) - render( , ) @@ -88,15 +98,13 @@ describe('AgentList', () => { }) it('only renders the delete button on custom agents owned by the user', () => { - const onToggle = mock(() => {}) - const onDelete = mock(() => {}) - render( , ) @@ -105,11 +113,39 @@ describe('AgentList', () => { expect(screen.getByTestId(`agent-delete-${customAgent.id}`)).toBeInTheDocument() }) + it('only renders the edit button on custom agents owned by the user', () => { + render( + , + ) + + expect(screen.queryByTestId(`agent-edit-${builtInAgent.id}`)).not.toBeInTheDocument() + expect(screen.queryByTestId(`agent-edit-${systemAgent.id}`)).not.toBeInTheDocument() + expect(screen.getByTestId(`agent-edit-${customAgent.id}`)).toBeInTheDocument() + }) + + it('calls onEdit with the agent when the edit button is clicked', () => { + const onEdit = mock<(agent: Agent) => void>(() => {}) + + render() + + fireEvent.click(screen.getByTestId(`agent-edit-${customAgent.id}`)) + + expect(onEdit).toHaveBeenCalledTimes(1) + expect(onEdit.mock.calls[0][0].id).toBe(customAgent.id) + }) + it('calls onToggle with the new enabled value when a custom agent toggle flips', () => { const onToggle = mock<(agent: Agent, enabled: boolean) => void>(() => {}) - const onDelete = mock<(agent: Agent) => void>(() => {}) - render() + render( + , + ) const toggle = screen.getByTestId(`agent-toggle-${customAgent.id}`) fireEvent.click(toggle) @@ -121,28 +157,19 @@ describe('AgentList', () => { }) it('disables the toggle for the built-in agent', () => { - const onToggle = mock(() => {}) - const onDelete = mock(() => {}) - - render() + render() expect(screen.getByTestId(`agent-toggle-${builtInAgent.id}`)).toBeDisabled() }) it('disables the toggle for system agents', () => { - const onToggle = mock(() => {}) - const onDelete = mock(() => {}) - - render() + render() expect(screen.getByTestId(`agent-toggle-${systemAgent.id}`)).toBeDisabled() }) it('keeps the toggle enabled for custom agents', () => { - const onToggle = mock(() => {}) - const onDelete = mock(() => {}) - - render() + render() expect(screen.getByTestId(`agent-toggle-${customAgent.id}`)).not.toBeDisabled() }) diff --git a/src/components/settings/agents/agent-list.tsx b/src/components/settings/agents/agent-list.tsx index 0177078fb..788b7cf86 100644 --- a/src/components/settings/agents/agent-list.tsx +++ b/src/components/settings/agents/agent-list.tsx @@ -9,16 +9,24 @@ type AgentListProps = { agents: Agent[] currentUserId: string | null onToggle: (agent: Agent, enabled: boolean) => void + onEdit: (agent: Agent) => void onDelete: (agent: Agent) => void } /** Renders the unified agent list returned by `useAllAgents` (built-in first, * then system, then user customs). Composition lives in the DAL — this * component is purely visual + event-dispatching so it stays trivial to test. */ -export const AgentList = ({ agents, currentUserId, onToggle, onDelete }: AgentListProps) => ( +export const AgentList = ({ agents, currentUserId, onToggle, onEdit, onDelete }: AgentListProps) => (
{agents.map((agent) => ( - + ))}
) diff --git a/src/components/settings/agents/agent-row.tsx b/src/components/settings/agents/agent-row.tsx index a6da5f8cf..6539f1153 100644 --- a/src/components/settings/agents/agent-row.tsx +++ b/src/components/settings/agents/agent-row.tsx @@ -8,7 +8,7 @@ import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { useState } from 'react' -import { Globe, Server, Trash2, Zap } from 'lucide-react' +import { Globe, Pencil, Server, Trash2, Zap } from 'lucide-react' import type { Agent } from '@/types/acp' /** Visual order: built-in (zap) → managed/system (server) → remote (globe). */ @@ -50,6 +50,12 @@ export const canDeleteAgent = (agent: Agent, currentUserId: string | null): bool return agent.userId === currentUserId } +/** Predicate for the edit action's visibility. Mirrors `canDeleteAgent`: + * built-in is in-code, system agents are managed via env vars, and customs + * belong to the user who created them. */ +export const canEditAgent = (agent: Agent, currentUserId: string | null): boolean => + canDeleteAgent(agent, currentUserId) + /** Computes the toggle's disabled state and the corresponding "always available" * tooltip text. Built-in is an in-code constant; system agents are configured * on the backend via env vars — neither can be toggled by the user. Exported @@ -68,12 +74,14 @@ type AgentRowProps = { agent: Agent currentUserId: string | null onToggle: (agent: Agent, enabled: boolean) => void + onEdit: (agent: Agent) => void onDelete: (agent: Agent) => void } -export const AgentRow = ({ agent, currentUserId, onToggle, onDelete }: AgentRowProps) => { +export const AgentRow = ({ agent, currentUserId, onToggle, onEdit, onDelete }: AgentRowProps) => { const Icon = iconForAgent(agent) const badge = badgeForAgent(agent) + const showEdit = canEditAgent(agent, currentUserId) const showDelete = canDeleteAgent(agent, currentUserId) const { disabled: toggleDisabled, disabledTooltip } = agentToggleDisabled(agent) const isEnabled = agent.enabled === 1 @@ -121,6 +129,25 @@ export const AgentRow = ({ agent, currentUserId, onToggle, onDelete }: AgentRowP

{disabledTooltip ?? (isEnabled ? 'Disable agent' : 'Enable agent')}

+ {showEdit && ( + + + + + +

Edit agent

+ + + )} {showDelete && ( diff --git a/src/routes/settings/agents/index.tsx b/src/routes/settings/agents/index.tsx index 012457db4..8d898c192 100644 --- a/src/routes/settings/agents/index.tsx +++ b/src/routes/settings/agents/index.tsx @@ -43,6 +43,9 @@ export default function AgentsSettingsPage({ isStandalone }: AgentsSettingsPageP const allowCustomAgents = useConfigStore((state) => selectAllowCustomAgents(state.config)) const [dialogOpen, setDialogOpen] = useState(false) + // `null` ⇒ Add mode; an Agent ⇒ Edit mode. The dialog receives a `key` + // derived from the agent id so its reducer remounts when switching targets. + const [editingAgent, setEditingAgent] = useState(null) // Defence against direct URL / bookmark when the entry is hidden in the // sidebar. Anonymous users behind the proxy can't reach managed agents, so @@ -69,7 +72,23 @@ export default function AgentsSettingsPage({ isStandalone }: AgentsSettingsPageP await deleteAgent(db, agent.id) } + const handleEdit = (agent: Agent) => { + setEditingAgent(agent) + setDialogOpen(true) + } + const handleSubmit = async (payload: AddCustomAgentPayload) => { + if (editingAgent) { + // Only customs are editable; system / built-in rows never reach this + // path (the row hides the Edit affordance). + await updateAgent(db, editingAgent.id, { + name: payload.name, + transport: payload.transport, + url: payload.url, + description: payload.description, + }) + return + } if (!currentUserId) { // Anonymous sessions can't sync custom agents — the page hides the // dialog trigger in that case, but the guard keeps the write safe. @@ -87,6 +106,14 @@ export default function AgentsSettingsPage({ isStandalone }: AgentsSettingsPageP }) } + const handleDialogOpenChange = (next: boolean) => { + setDialogOpen(next) + if (!next) { + // Drop the edit target so a follow-up "+" opens a fresh Add dialog. + setEditingAgent(null) + } + } + return (
@@ -96,7 +123,10 @@ export default function AgentsSettingsPage({ isStandalone }: AgentsSettingsPageP size="icon" className="rounded-lg" aria-label="Add Custom Agent" - onClick={() => setDialogOpen(true)} + onClick={() => { + setEditingAgent(null) + setDialogOpen(true) + }} disabled={!currentUserId} > @@ -104,12 +134,20 @@ export default function AgentsSettingsPage({ isStandalone }: AgentsSettingsPageP )} - +