diff --git a/desktop/src/api/agents.test.ts b/desktop/src/api/agents.test.ts index 415ab20b..2df6f1fa 100644 --- a/desktop/src/api/agents.test.ts +++ b/desktop/src/api/agents.test.ts @@ -67,6 +67,32 @@ describe('agentsApi', () => { expect(apiDeleteMock).toHaveBeenCalledWith('/api/agents/reviewer?scope=user') }) + it('sends an override to the sub-resource, keeping explicit nulls', () => { + // `null` is the wire form of "clear this field"; dropping it would silently + // turn a reset into a no-op. + agentsApi.setOverride('Explore', { cwd: '/workspace/one', model: null, effort: 'low' }) + + expect(apiPutMock).toHaveBeenCalledWith('/api/agents/Explore/override', { + cwd: '/workspace/one', + model: null, + effort: 'low', + }) + }) + + it('URL-encodes the override clear path and cwd', () => { + agentsApi.clearOverride('reviewer/name?', '/workspace/project one') + + expect(apiDeleteMock).toHaveBeenCalledWith( + '/api/agents/reviewer%2Fname%3F/override?cwd=%2Fworkspace%2Fproject+one', + ) + }) + + it('omits an empty cwd from an override clear', () => { + agentsApi.clearOverride('Explore') + + expect(apiDeleteMock).toHaveBeenCalledWith('/api/agents/Explore/override') + }) + it('reloads the exact active session with the control timeout', () => { agentsApi.reload('session/one?') diff --git a/desktop/src/api/agents.ts b/desktop/src/api/agents.ts index 05c00d50..ae638c1d 100644 --- a/desktop/src/api/agents.ts +++ b/desktop/src/api/agents.ts @@ -23,7 +23,24 @@ export type AgentDefinition = { target?: string overriddenBy?: AgentSource isActive: boolean + /** The backing file can be rewritten. Never true for built-in agents. */ editable?: boolean + /** Built-in agents only: model and effort can be changed via setOverride. */ + overridable?: boolean + /** + * Built-in agents only: what this build ships with, so the UI can name and + * restore the default. Never hardcode it — it varies per agent and per build. + */ + defaults?: { model?: string; effort?: string | number } + /** Built-in agents only: the override currently in effect, if any. */ + override?: { model?: string; effort?: string | number; source: AgentSource } +} + +/** `null` clears that field; an omitted field is left unchanged. */ +export type AgentOverrideInput = { + cwd?: string + model?: string | null + effort?: string | number | null } export type AgentScope = 'user' | 'project' @@ -82,6 +99,19 @@ export const agentsApi = { if (target) query.set('target', target) return api.delete(`/api/agents/${encodeURIComponent(name)}?${query.toString()}`) }, + setOverride: (name: string, input: AgentOverrideInput) => + api.put( + `/api/agents/${encodeURIComponent(name)}/override`, + input, + ), + clearOverride: (name: string, cwd?: string) => { + const query = new URLSearchParams() + if (cwd) query.set('cwd', cwd) + const suffix = query.toString() ? `?${query.toString()}` : '' + return api.delete( + `/api/agents/${encodeURIComponent(name)}/override${suffix}`, + ) + }, reload: (sessionId: string) => api.post( `/api/agents/reload?sessionId=${encodeURIComponent(sessionId)}`, diff --git a/desktop/src/components/settings/AgentManager.test.tsx b/desktop/src/components/settings/AgentManager.test.tsx index cfbd3b15..ae47326a 100644 --- a/desktop/src/components/settings/AgentManager.test.tsx +++ b/desktop/src/components/settings/AgentManager.test.tsx @@ -7,6 +7,8 @@ const apiCreateMock = vi.hoisted(() => vi.fn()) const apiUpdateMock = vi.hoisted(() => vi.fn()) const apiDeleteMock = vi.hoisted(() => vi.fn()) const apiReloadMock = vi.hoisted(() => vi.fn()) +const apiSetOverrideMock = vi.hoisted(() => vi.fn()) +const apiClearOverrideMock = vi.hoisted(() => vi.fn()) const recentProjectsMock = vi.hoisted(() => vi.fn()) vi.mock('../../api/agents', async (importOriginal) => { @@ -19,6 +21,8 @@ vi.mock('../../api/agents', async (importOriginal) => { update: apiUpdateMock, delete: apiDeleteMock, reload: apiReloadMock, + setOverride: apiSetOverrideMock, + clearOverride: apiClearOverrideMock, }, } }) @@ -65,6 +69,23 @@ function makeAgent(overrides: Partial = {}): AgentDefinition { } } +function makeBuiltInAgent(overrides: Partial = {}): AgentDefinition { + return { + agentType: 'Explore', + description: 'Explore the codebase', + source: 'built-in', + baseDir: 'built-in', + isActive: true, + // Built-ins are never file-editable; only model and effort can change. + editable: false, + overridable: true, + defaults: { model: 'haiku' }, + model: 'haiku', + modelDisplay: 'haiku', + ...overrides, + } +} + function setProjectSession(cwd?: string) { useSessionStore.setState({ sessions: cwd ? [{ @@ -612,6 +633,204 @@ describe('AgentManager', () => { expect(screen.getByRole('dialog', { name: 'Create Agent' })).toBeInTheDocument() }) + it('offers edit and delete on the row itself without nesting buttons', async () => { + const agent = makeAgent() + await renderManager({ activeAgents: [agent], allAgents: [agent] }) + + const editButton = screen.getByRole('button', { name: 'Edit code_reviewer' }) + expect(editButton).toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Delete code_reviewer' })).toBeInTheDocument() + + // The structural assertion is the one that matters: nested buttons still + // render and still fire in jsdom, so behaviour alone cannot catch them. + const row = editButton.closest('div.group') + expect(row).not.toBeNull() + expect(row!.querySelector('button button')).toBeNull() + }) + + it('keeps the row body clickable now that it is no longer the outer element', async () => { + const agent = makeAgent() + await renderManager({ activeAgents: [agent], allAgents: [agent] }) + + fireEvent.click(screen.getByText('code_reviewer')) + + expect(await screen.findByRole('button', { name: 'Back to list' })).toBeInTheDocument() + expect(useAgentStore.getState().selectedAgent?.agentType).toBe('code_reviewer') + }) + + it('deletes straight from the row with that row exact target', async () => { + const agent = makeAgent({ source: 'projectSettings' }) + apiListMock + .mockResolvedValueOnce({ activeAgents: [agent], allAgents: [agent] }) + .mockResolvedValueOnce(EMPTY_RESPONSE) + apiDeleteMock.mockResolvedValue(undefined) + + render() + await waitFor(() => expect(apiListMock).toHaveBeenCalledTimes(1)) + fireEvent.click(screen.getByRole('button', { name: 'Delete code_reviewer' })) + fireEvent.click(screen.getByRole('button', { name: 'Delete Agent' })) + + await waitFor(() => expect(apiDeleteMock).toHaveBeenCalledWith( + 'code_reviewer', + 'project', + '/workspace/project', + 'nested/custom-agent-file.md', + )) + }) + + it('offers no row actions on sources that can be neither edited nor overridden', async () => { + const plugin = makeAgent({ + agentType: 'plugin_agent', + source: 'plugin', + editable: false, + target: undefined, + }) + await renderManager({ activeAgents: [plugin], allAgents: [plugin] }) + + expect(screen.queryByRole('button', { name: 'Edit plugin_agent' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Delete plugin_agent' })).toBeNull() + expect(screen.queryByRole('button', { name: /Adjust the model/ })).toBeNull() + }) + + it('offers a built-in row model adjustment but never a delete', async () => { + const builtIn = makeBuiltInAgent() + await renderManager({ activeAgents: [builtIn], allAgents: [builtIn] }) + + expect( + screen.getByRole('button', { name: 'Adjust the model and effort for Explore' }), + ).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Delete Explore' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Edit Explore' })).toBeNull() + }) + + it('separates the built-in default from inherit and sends null for the default', async () => { + const builtIn = makeBuiltInAgent() + await renderManager({ activeAgents: [builtIn], allAgents: [builtIn] }) + apiSetOverrideMock.mockResolvedValue({ agent: builtIn }) + apiListMock.mockResolvedValue({ activeAgents: [builtIn], allAgents: [builtIn] }) + + fireEvent.click( + screen.getByRole('button', { name: 'Adjust the model and effort for Explore' }), + ) + + // Both entries must exist. For Explore the shipped default is haiku while + // inherit means "follow the main session" — collapsing them would make + // inherit unreachable, and the default label is read from the server. + fireEvent.click(screen.getByRole('button', { name: 'Model' })) + expect(screen.getByRole('option', { name: 'Built-in default (haiku)' })).toBeInTheDocument() + expect(screen.getByRole('option', { name: 'Inherit from parent' })).toBeInTheDocument() + fireEvent.click(screen.getByRole('option', { name: 'Built-in default (haiku)' })) + + chooseAgentSelect('Reasoning effort', 'high') + fireEvent.click(screen.getByRole('button', { name: 'Save' })) + + // `null`, never the literal 'haiku': writing today's default into + // settings.json would freeze it there forever. + await waitFor(() => expect(apiSetOverrideMock).toHaveBeenCalledWith('Explore', { + cwd: '/workspace/project', + model: null, + effort: 'high', + })) + }) + + it('sends inherit as a real value when the user picks it', async () => { + const builtIn = makeBuiltInAgent() + await renderManager({ activeAgents: [builtIn], allAgents: [builtIn] }) + apiSetOverrideMock.mockResolvedValue({ agent: builtIn }) + apiListMock.mockResolvedValue({ activeAgents: [builtIn], allAgents: [builtIn] }) + + fireEvent.click( + screen.getByRole('button', { name: 'Adjust the model and effort for Explore' }), + ) + chooseAgentSelect('Model', 'Inherit from parent') + fireEvent.click(screen.getByRole('button', { name: 'Save' })) + + await waitFor(() => expect(apiSetOverrideMock).toHaveBeenCalledWith('Explore', { + cwd: '/workspace/project', + model: 'inherit', + effort: null, + })) + }) + + it('resets a built-in through the server instead of writing the default back', async () => { + const overridden = makeBuiltInAgent({ + model: 'sonnet', + modelDisplay: 'sonnet', + override: { model: 'sonnet', source: 'userSettings' }, + }) + await renderManager({ activeAgents: [overridden], allAgents: [overridden] }) + apiClearOverrideMock.mockResolvedValue({ agent: makeBuiltInAgent() }) + apiListMock.mockResolvedValue({ activeAgents: [overridden], allAgents: [overridden] }) + + fireEvent.click( + screen.getByRole('button', { name: 'Adjust the model and effort for Explore' }), + ) + fireEvent.click(screen.getByRole('button', { name: 'Reset to built-in default' })) + + await waitFor(() => expect(apiClearOverrideMock).toHaveBeenCalledWith( + 'Explore', + '/workspace/project', + )) + // Reset must clear the setting, not write the current default back as a + // value — that would pin today's default into settings.json permanently. + expect(apiSetOverrideMock).not.toHaveBeenCalled() + // A running session caches agent definitions, so the write alone is not + // enough for the change to take effect. + expect(apiReloadMock).toHaveBeenCalledWith('session-1') + }) + + it('locks the controls when the override comes from managed settings', async () => { + const managed = makeBuiltInAgent({ + model: 'opus', + override: { model: 'opus', source: 'policySettings' }, + }) + await renderManager({ activeAgents: [managed], allAgents: [managed] }) + + fireEvent.click( + screen.getByRole('button', { name: 'Adjust the model and effort for Explore' }), + ) + + expect(screen.getByRole('button', { name: 'Model' })).toBeDisabled() + expect(screen.getByRole('button', { name: 'Reasoning effort' })).toBeDisabled() + expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled() + // Resetting would write to the user file, which cannot win over a policy. + expect(screen.queryByRole('button', { name: 'Reset to built-in default' })).toBeNull() + expect(screen.getByRole('status')).toHaveTextContent( + 'Set by Managed settings and not editable here.', + ) + }) + + it('warns that a shadowed built-in will not take effect yet', async () => { + // Editing a built-in that a same-named user agent shadows would look like + // it worked and change nothing at spawn time. + const shadowed = makeBuiltInAgent({ overriddenBy: 'userSettings', isActive: false }) + await renderManager({ activeAgents: [], allAgents: [shadowed] }) + + fireEvent.click( + screen.getByRole('button', { name: 'Adjust the model and effort for Explore' }), + ) + + expect(screen.getByRole('status')).toHaveTextContent( + 'An agent of the same name from User is active', + ) + }) + + it('keeps the override modal open when saving fails', async () => { + const builtIn = makeBuiltInAgent() + await renderManager({ activeAgents: [builtIn], allAgents: [builtIn] }) + apiSetOverrideMock.mockRejectedValue(new Error('AGENT_CUSTOMIZATION_LOCKED')) + + fireEvent.click( + screen.getByRole('button', { name: 'Adjust the model and effort for Explore' }), + ) + chooseAgentSelect('Model', 'sonnet') + fireEvent.click(screen.getByRole('button', { name: 'Save' })) + + expect(await screen.findByRole('alert')).toHaveTextContent('Failed to save the override') + expect(screen.getByRole('alert')).not.toHaveTextContent('AGENT_CUSTOMIZATION_LOCKED') + expect(screen.getByRole('dialog', { name: 'Adjust built-in agent' })).toBeInTheDocument() + }) + it('localizes load failures without exposing raw server errors', async () => { useSettingsStore.setState({ locale: 'zh' }) apiListMock.mockRejectedValue(new Error('HTTP 500: internal agent path leaked')) diff --git a/desktop/src/components/settings/AgentManager.tsx b/desktop/src/components/settings/AgentManager.tsx index 99b3abec..feade3e0 100644 --- a/desktop/src/components/settings/AgentManager.tsx +++ b/desktop/src/components/settings/AgentManager.tsx @@ -43,6 +43,7 @@ import { ErrorState } from '@/components/ui/ErrorState' import { LoadingState } from '@/components/ui/LoadingState' import { DirectoryPicker } from '@/components/composite/DirectoryPicker' import { Dropdown } from '@/components/ui/Dropdown' +import { IconButton } from '@/components/ui/IconButton' import { Input } from '@/components/ui/Input' import { Modal } from '@/components/ui/Modal' import { SearchField } from '@/components/ui/SearchField' @@ -71,6 +72,11 @@ const AGENT_SOURCE_ORDER: AgentSource[] = [ const BUILT_IN_MODELS = ['haiku', 'sonnet', 'opus', 'fable'] as const const EFFORTS = ['low', 'medium', 'high', 'xhigh', 'max'] as const +/** + * "Use whatever this build ships" in the built-in override modal, submitted as + * `null`. Distinct from `inherit`, which is itself a persistable choice. + */ +const DEFAULT_CHOICE = '__default__' const NAME_PATTERN = /^[a-z0-9](?:[a-z0-9_-]{0,62}[a-z0-9])?$/ type ToolAccessMode = 'inherit' | 'none' | 'custom' type ToolCategory = 'readSearch' | 'modify' | 'execute' | 'workflow' | 'other' @@ -123,6 +129,7 @@ export function AgentManager() { const t = useTranslation() const [formState, setFormState] = useState<{ mode: 'create' | 'edit'; agent?: AgentDefinition } | null>(null) const [deleteTarget, setDeleteTarget] = useState(null) + const [overrideTarget, setOverrideTarget] = useState(null) const activeSession = sessions.find((session) => session.id === activeSessionId) const currentWorkDir = getSessionBrowsablePath(activeSession) @@ -185,6 +192,7 @@ export function AgentManager() { onBack={handleAgentBack} onEdit={() => setFormState({ mode: 'edit', agent: selectedAgent })} onDelete={() => setDeleteTarget(selectedAgent)} + onOverride={() => setOverrideTarget(selectedAgent)} /> ) : ( <> @@ -260,11 +268,18 @@ export function AgentManager() {
{group.map((agent, index) => ( -
- + + setFormState({ mode: 'edit', agent })} + onDelete={() => setDeleteTarget(agent)} + onOverride={() => setOverrideTarget(agent)} + /> + ))} @@ -326,6 +348,14 @@ export function AgentManager() { sessionId={contextSessionId} onClose={() => setDeleteTarget(null)} /> + {overrideTarget && ( + setOverrideTarget(null)} + /> + )} ) } @@ -335,11 +365,13 @@ function AgentDetailView({ onBack, onEdit, onDelete, + onOverride, }: { agent: AgentDefinition onBack: () => void onEdit: () => void onDelete: () => void + onOverride: () => void }) { const t = useTranslation() const sourceLabel = t(`settings.agents.source.${agent.source}`) @@ -362,7 +394,16 @@ function AgentDetailView({ ) : ( - {t('settings.agents.readOnly')} +
+ {agent.overridable && ( + + )} + {/* Kept alongside the button: the prompt and tools really are fixed, + and only the model and effort are not. */} + {t('settings.agents.readOnly')} +
)} @@ -938,6 +979,272 @@ function AgentDeleteDialog({ ) } +/** + * Model/effort editor for a built-in agent. + * + * Deliberately not `AgentFormModal` with a flag. That component exists to build + * an `AgentMutationInput` whose name, description and system prompt are all + * required, and none of those apply here; threading a variant through its + * render branches and its payload-construction chain would put the riskiest + * code in this file on a second, unrelated path. + */ +function BuiltInAgentOverrideModal({ + agent, + cwd, + sessionId, + onClose, +}: { + agent: AgentDefinition + cwd?: string + sessionId?: string + onClose: () => void +}) { + const t = useTranslation() + const setAgentOverride = useAgentStore((state) => state.setAgentOverride) + const clearAgentOverride = useAgentStore((state) => state.clearAgentOverride) + const isMutating = useAgentStore((state) => state.isMutating) + + const defaultModel = agent.defaults?.model + const defaultEffort = agent.defaults?.effort + const overrideSource = agent.override?.source + // A managed or project-level override cannot be edited from the user file + // this modal writes to, so saying so beats a write that silently loses. + const isManaged = overrideSource !== undefined && overrideSource !== 'userSettings' + + const initialModel = agent.override?.model + const initialEffort = agent.override?.effort + const [modelChoice, setModelChoice] = useState( + initialModel === undefined + ? DEFAULT_CHOICE + : initialModel === 'inherit' || BUILT_IN_MODELS.includes(initialModel as typeof BUILT_IN_MODELS[number]) + ? initialModel + : 'custom', + ) + const [customModel, setCustomModel] = useState( + modelChoice === 'custom' ? (initialModel ?? '') : '', + ) + const [effort, setEffort] = useState( + initialEffort === undefined ? DEFAULT_CHOICE : String(initialEffort), + ) + const [customModelError, setCustomModelError] = useState(null) + const [submitError, setSubmitError] = useState(null) + + const describeDefault = (value: string | number | undefined) => + value === undefined + ? t('settings.agents.overrideDefaultNone') + : t('settings.agents.overrideDefault', { value: String(value) }) + + const handleSave = async () => { + if (modelChoice === 'custom' && !customModel.trim()) { + setCustomModelError(t('settings.agents.form.customModelRequired')) + return + } + setCustomModelError(null) + setSubmitError(null) + try { + await setAgentOverride( + agent.agentType, + { + ...(cwd ? { cwd } : {}), + // `null` clears the override so the shipped default applies again. + // Never send the default's literal value: that would freeze today's + // default into the user's settings file forever. + model: + modelChoice === DEFAULT_CHOICE + ? null + : modelChoice === 'custom' + ? customModel.trim() + : modelChoice, + effort: effort === DEFAULT_CHOICE ? null : effort, + }, + sessionId, + ) + onClose() + } catch { + setSubmitError(t('settings.agents.overrideSaveError')) + } + } + + const handleReset = async () => { + setSubmitError(null) + try { + await clearAgentOverride(agent.agentType, cwd, sessionId) + onClose() + } catch { + setSubmitError(t('settings.agents.overrideResetError')) + } + } + + return ( + {} : onClose} + title={t('settings.agents.overrideTitle')} + width={520} + footer={( + <> + {agent.override && !isManaged && ( + + )} + + + + )} + > +
+
+ + {agent.agentType} + + {t('settings.agents.source.built-in')} + {agent.override && {t('settings.agents.overrideBadge')}} +
+ + {agent.overriddenBy && ( + // Editing a built-in that a same-named user agent shadows would look + // like it worked and change nothing at spawn time. +

+ {t('settings.agents.overrideShadowed', { + source: t(`settings.agents.source.${agent.overriddenBy}`), + })} +

+ )} + {isManaged && ( +

+ {t('settings.agents.overrideManaged', { + source: t(`settings.agents.source.${overrideSource}`), + })} +

+ )} + +
+ + ({ value: model, label: model })), + { value: 'custom', label: t('settings.agents.form.customModel') }, + ]} + /> + + + ({ value, label: value })), + ]} + /> + +
+ + {modelChoice === 'custom' && ( + setCustomModel(event.target.value)} + /> + )} + +

+ {t('settings.agents.overrideHint')} +

+

+ {t('settings.agents.overrideScopeHint')} +

+ {submitError &&

{submitError}

} +
+
+ ) +} + +/** + * The per-row actions, rendered as a sibling of the row's primary button. + * + * Hidden until the row is hovered, but `focus-within` is not optional: without + * it a keyboard user tabs onto a control they cannot see. The fade lives on + * this wrapper rather than on the buttons because IconButton already sets + * `transition-colors`, and a second transition utility on the same element + * resolves by stylesheet order instead of by intent. + */ +function AgentRowActions({ + agent, + onEdit, + onDelete, + onOverride, +}: { + agent: AgentDefinition + onEdit: () => void + onDelete: () => void + onOverride: () => void +}) { + const t = useTranslation() + const editable = isEditableAgent(agent) + const overridable = agent.overridable === true + + if (!editable && !overridable) return null + + return ( + + {editable ? ( + <> + } + label={t('settings.agents.rowEdit', { name: agent.agentType })} + onClick={onEdit} + /> + } + label={t('settings.agents.rowDelete', { name: agent.agentType })} + onClick={onDelete} + /> + + ) : ( + // Built-ins get model/effort only — their file is never rewritten, so + // there is deliberately no delete here. + } + label={t('settings.agents.rowOverride', { name: agent.agentType })} + onClick={onOverride} + /> + )} + + ) +} + function isEditableAgent(agent: AgentDefinition) { return agent.editable === true && getEditableScope(agent) !== null } @@ -1011,11 +1318,13 @@ function AgentSelect({ items, value, onChange, + disabled, }: { label: string items: Array<{ value: T; label: string; icon?: ReactNode }> value: T onChange: (value: T) => void + disabled?: boolean }) { const selected = items.find((item) => item.value === value) ?? items[0] return ( @@ -1031,7 +1340,8 @@ function AgentSelect({