fix(desktop): keep the Kanban panel and toolset in agreement on a failed write
The Desktop switch flipped the panel before the toolset PUT, so a rejected write left panel on / tools off (or the reverse) with only a toast. Write the toolset first and move the panel only to what the backend holds: on a rejection re-read the toolsets list (a timeout may still have committed) and follow it; if the re-read also fails, leave the panel untouched. The switch is disabled while the write is in flight and live again for a retry.
This commit is contained in:
Hermes Agent
brooklyn!
@@ -8,6 +8,7 @@ import { $pluginRecords } from '@/contrib/plugins-store'
|
||||
import { queryClient } from '@/lib/query-client'
|
||||
import { $agentPlugins, $agentPluginsStatus, type AgentPluginRow } from '@/store/agent-plugins'
|
||||
import { $confirmRequest, settleConfirm } from '@/store/confirm'
|
||||
import { $notifications } from '@/store/notifications'
|
||||
import { $pluginInstallRequest, closePluginInstallRequest } from '@/store/plugin-install-request'
|
||||
import { $connection } from '@/store/session'
|
||||
|
||||
@@ -85,10 +86,12 @@ vi.mock('@/contrib/runtime-loader', async importOriginal => ({
|
||||
|
||||
// #96969: a Desktop switch whose feature also ships an agent toolset must
|
||||
// reach the same PUT /api/tools/toolsets/{name} the Toolsets tab uses.
|
||||
const setToolsetEnabled = vi.fn(async () => ({ enabled: true, name: 'kanban', ok: true }))
|
||||
const setToolsetEnabled = vi.fn(async (..._args: unknown[]) => ({ enabled: true, name: 'kanban', ok: true }))
|
||||
const getToolsets = vi.fn(async (..._args: unknown[]): Promise<{ enabled: boolean; name: string }[]> => [])
|
||||
|
||||
vi.mock('@/api/toolsets', async importOriginal => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
getToolsets: (...args: unknown[]) => getToolsets(...args),
|
||||
setToolsetEnabled: (...args: unknown[]) => setToolsetEnabled(...(args as Parameters<typeof setToolsetEnabled>))
|
||||
}))
|
||||
|
||||
@@ -102,7 +105,10 @@ beforeEach(() => {
|
||||
requestGateway.mockReset()
|
||||
requestGateway.mockImplementation(async () => ({ plugins: $agentPlugins.get() }))
|
||||
setEnvVar.mockClear()
|
||||
setToolsetEnabled.mockClear()
|
||||
setToolsetEnabled.mockReset()
|
||||
setToolsetEnabled.mockResolvedValue({ enabled: true, name: 'kanban', ok: true })
|
||||
getToolsets.mockReset()
|
||||
getToolsets.mockResolvedValue([])
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -421,6 +427,160 @@ describe('PluginsTab', () => {
|
||||
sha: entry.sha
|
||||
})
|
||||
})
|
||||
|
||||
// #96969: the Desktop Kanban switch enabled only this app's UI panel; the
|
||||
// agent-side kanban toolset stayed off, so the board existed but the agent
|
||||
// had no kanban tools. The switch must flip both, scoped to the profile the
|
||||
// page shows (the toolset is per-profile config), and refresh the Toolsets
|
||||
// tab's cache so its row repaints.
|
||||
it('enables the kanban agent toolset when the Desktop panel is switched on', async () => {
|
||||
const invalidate = vi.spyOn(queryClient, 'invalidateQueries')
|
||||
// Panel currently off: clicking the switch turns it on.
|
||||
$pluginRecords.set({
|
||||
kanban: { id: 'kanban', name: 'Kanban', kind: 'bundled', status: 'disabled' }
|
||||
})
|
||||
|
||||
renderPlugins({ profile: 'workbot', scopeLabel: 'workbot' })
|
||||
|
||||
screen.getByRole('switch', { name: 'Desktop: Kanban' }).click()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setToolsetEnabled).toHaveBeenCalledWith('kanban', true, 'workbot')
|
||||
})
|
||||
expect(invalidate).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['toolsets-list'] }))
|
||||
invalidate.mockRestore()
|
||||
})
|
||||
|
||||
it('disables the kanban agent toolset when the Desktop panel is switched off', async () => {
|
||||
// Panel currently on: clicking the switch turns it off.
|
||||
$pluginRecords.set({
|
||||
kanban: { id: 'kanban', name: 'Kanban', kind: 'bundled', status: 'loaded' }
|
||||
})
|
||||
|
||||
renderPlugins({ profile: 'workbot', scopeLabel: 'workbot' })
|
||||
|
||||
screen.getByRole('switch', { name: 'Desktop: Kanban' }).click()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(setToolsetEnabled).toHaveBeenCalledWith('kanban', false, 'workbot')
|
||||
})
|
||||
})
|
||||
|
||||
describe('Desktop Kanban switch when the toolset write fails', () => {
|
||||
// A real loader handle, so the switch's `checked` (desktop.status) and the
|
||||
// persisted decision genuinely move when the panel is toggled.
|
||||
const publishKanban = (status: 'disabled' | 'loaded') =>
|
||||
publishPlugin(
|
||||
{ id: 'kanban', name: 'Kanban', kind: 'bundled', status },
|
||||
{ activate: () => patchPlugin('kanban', { status: 'loaded' }), deactivate: () => undefined }
|
||||
)
|
||||
|
||||
const kanbanSwitch = () => screen.getByRole<HTMLButtonElement>('switch', { name: 'Desktop: Kanban' })
|
||||
const checked = () => kanbanSwitch().getAttribute('aria-checked')
|
||||
const notices = (kind: string) => $notifications.get().filter(n => n.kind === kind)
|
||||
const toolsetReads = (enabled: boolean) => [{ enabled, name: 'kanban' }]
|
||||
|
||||
// Resolves once the write attempt (and any re-read) has settled.
|
||||
const settled = () => waitFor(() => expect(kanbanSwitch().disabled).toBe(false))
|
||||
|
||||
beforeEach(() => {
|
||||
$pluginDecisions.set({})
|
||||
$notifications.set([])
|
||||
window.localStorage.clear()
|
||||
})
|
||||
|
||||
afterEach(() => dropPlugin('kanban'))
|
||||
|
||||
it('keeps panel and tools off when enabling is rejected, then retries', async () => {
|
||||
publishKanban('disabled')
|
||||
setToolsetEnabled.mockRejectedValueOnce(new Error('HTTP 500'))
|
||||
getToolsets.mockResolvedValueOnce(toolsetReads(false))
|
||||
|
||||
renderPlugins({ profile: 'workbot', scopeLabel: 'workbot' })
|
||||
fireEvent.click(kanbanSwitch())
|
||||
await waitFor(() => expect(notices('error')).toHaveLength(1))
|
||||
await settled()
|
||||
|
||||
expect(checked()).toBe('false')
|
||||
expect($pluginRecords.get().kanban.status).toBe('disabled')
|
||||
expect($pluginDecisions.get()).not.toHaveProperty('kanban')
|
||||
expect(window.localStorage.getItem('hermes.desktop.pluginDecisions.v2')).toBeNull()
|
||||
expect(notices('success')).toHaveLength(0)
|
||||
|
||||
// Same intended value again, backend healthy: both halves turn on.
|
||||
fireEvent.click(kanbanSwitch())
|
||||
await waitFor(() => expect(checked()).toBe('true'))
|
||||
|
||||
expect(setToolsetEnabled).toHaveBeenLastCalledWith('kanban', true, 'workbot')
|
||||
expect($pluginDecisions.get().kanban).toBe(true)
|
||||
expect(notices('success')).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('keeps panel and tools on when disabling is rejected, then retries', async () => {
|
||||
publishKanban('loaded')
|
||||
setToolsetEnabled.mockRejectedValueOnce(new Error('HTTP 500'))
|
||||
getToolsets.mockResolvedValueOnce(toolsetReads(true))
|
||||
|
||||
renderPlugins({ profile: 'workbot', scopeLabel: 'workbot' })
|
||||
fireEvent.click(kanbanSwitch())
|
||||
await waitFor(() => expect(notices('error')).toHaveLength(1))
|
||||
await settled()
|
||||
|
||||
expect(checked()).toBe('true')
|
||||
expect($pluginRecords.get().kanban.status).toBe('loaded')
|
||||
expect($pluginDecisions.get()).not.toHaveProperty('kanban')
|
||||
expect(notices('success')).toHaveLength(0)
|
||||
|
||||
fireEvent.click(kanbanSwitch())
|
||||
await waitFor(() => expect(checked()).toBe('false'))
|
||||
|
||||
expect(setToolsetEnabled).toHaveBeenLastCalledWith('kanban', false, 'workbot')
|
||||
expect($pluginDecisions.get().kanban).toBe(false)
|
||||
expect(notices('success')).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('follows the backend when a timed-out write actually committed', async () => {
|
||||
publishKanban('disabled')
|
||||
setToolsetEnabled.mockRejectedValueOnce(new Error('Request timed out'))
|
||||
getToolsets.mockResolvedValueOnce(toolsetReads(true))
|
||||
|
||||
renderPlugins({ profile: 'workbot', scopeLabel: 'workbot' })
|
||||
fireEvent.click(kanbanSwitch())
|
||||
await waitFor(() => expect(checked()).toBe('true'))
|
||||
|
||||
expect(getToolsets).toHaveBeenCalledWith('workbot')
|
||||
expect($pluginDecisions.get().kanban).toBe(true)
|
||||
expect(notices('error')).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('leaves the panel alone and stays retryable when the outcome is unknown', async () => {
|
||||
publishKanban('loaded')
|
||||
setToolsetEnabled.mockRejectedValueOnce(new Error('Request timed out'))
|
||||
getToolsets.mockRejectedValueOnce(new Error('Request timed out'))
|
||||
|
||||
renderPlugins({ profile: 'workbot', scopeLabel: 'workbot' })
|
||||
fireEvent.click(kanbanSwitch())
|
||||
await waitFor(() => expect(notices('error')).toHaveLength(1))
|
||||
await settled()
|
||||
|
||||
expect(checked()).toBe('true')
|
||||
expect($pluginRecords.get().kanban.status).toBe('loaded')
|
||||
expect($pluginDecisions.get()).not.toHaveProperty('kanban')
|
||||
expect(notices('success')).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
it('leaves toolsets alone for desktop plugins with no agent toolset', () => {
|
||||
$pluginRecords.set({
|
||||
media: { id: 'media', name: 'Media Studio', kind: 'disk', status: 'loaded' }
|
||||
})
|
||||
|
||||
renderPlugins({ profile: 'workbot', scopeLabel: 'workbot' })
|
||||
|
||||
screen.getByRole('switch', { name: 'Desktop: Media Studio' }).click()
|
||||
|
||||
expect(setToolsetEnabled).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('PluginsTab catalog UX', () => {
|
||||
|
||||
@@ -2,7 +2,7 @@ import { useStore } from '@nanostores/react'
|
||||
import { memo, type ReactNode, useCallback, useEffect, useMemo, useState } from 'react'
|
||||
|
||||
import { setEnvVar } from '@/api/config'
|
||||
import { setToolsetEnabled } from '@/api/toolsets'
|
||||
import { getToolsets, setToolsetEnabled } from '@/api/toolsets'
|
||||
import { useGatewayRequest } from '@/app/gateway/hooks/use-gateway-request'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Codicon } from '@/components/ui/codicon'
|
||||
@@ -229,6 +229,7 @@ function PackageRow({
|
||||
const settingsFields = agent?.settings_schema ?? []
|
||||
const hasSettings = Boolean(agent?.key) && settingsFields.length > 0
|
||||
const [settingsOpen, setSettingsOpen] = useState(false)
|
||||
const [desktopBusy, setDesktopBusy] = useState(false)
|
||||
const desktopOn = desktop ? desktop.status !== 'disabled' : false
|
||||
const agentOn = agent?.status === 'enabled'
|
||||
const agentToggleable = Boolean(agent?.key)
|
||||
@@ -245,6 +246,55 @@ function PackageRow({
|
||||
// package installed on a remote backend can never materialize here (#114079).
|
||||
const remoteBackend = useStore($connection)?.mode === 'remote'
|
||||
|
||||
// #96969: when the feature's agent-side tools live in a toolset (the
|
||||
// built-in Kanban board has no agent-plugin half), the Desktop switch flips
|
||||
// that per-profile opt-in with the panel, through the same
|
||||
// PUT /api/tools/toolsets/{name} the Toolsets tab uses. The backend write
|
||||
// goes first and the panel follows what the backend actually holds, so a
|
||||
// failed write can't leave the two halves disagreeing. A rejected PUT may
|
||||
// still have committed (a timeout leaves it unknown), so re-read instead of
|
||||
// assuming a rollback; if even that fails the panel stays put and the
|
||||
// switch is live again for a retry.
|
||||
const toggleDesktop = async (id: string, on: boolean) => {
|
||||
const toolset = DESKTOP_PLUGIN_TOOLSETS[id]
|
||||
|
||||
if (!toolset) {
|
||||
return setPluginEnabled(id, on)
|
||||
}
|
||||
|
||||
setDesktopBusy(true)
|
||||
|
||||
try {
|
||||
let toolsetOn: boolean | undefined
|
||||
|
||||
try {
|
||||
await setToolsetEnabled(toolset, on, profile)
|
||||
toolsetOn = on
|
||||
} catch (err) {
|
||||
toolsetOn = await getToolsets(profile).then(
|
||||
list => list.find(row => row.name === toolset)?.enabled,
|
||||
() => undefined
|
||||
)
|
||||
|
||||
if (toolsetOn !== on) {
|
||||
notifyError(err, p.toolsetToggleFailed(pkg.name))
|
||||
}
|
||||
}
|
||||
|
||||
void queryClient.invalidateQueries({ queryKey: TOOLSETS_QUERY_KEY })
|
||||
|
||||
if (toolsetOn === on) {
|
||||
await setPluginEnabled(id, on)
|
||||
notify({
|
||||
kind: 'success',
|
||||
message: on ? p.toolsetOn(pkg.name, scopeLabel) : p.toolsetOff(pkg.name, scopeLabel)
|
||||
})
|
||||
}
|
||||
} finally {
|
||||
setDesktopBusy(false)
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<div
|
||||
@@ -360,35 +410,10 @@ function PackageRow({
|
||||
<Switch
|
||||
aria-label={`${p.halfDesktop}: ${pkg.name}`}
|
||||
checked={desktopOn}
|
||||
disabled={desktopBusy}
|
||||
onCheckedChange={on => {
|
||||
triggerHaptic('selection')
|
||||
|
||||
void (async () => {
|
||||
await setPluginEnabled(desktop.id, on)
|
||||
// #96969: when the feature's agent-side tools live in a
|
||||
// toolset (the built-in Kanban board has no agent-plugin
|
||||
// half), flip that opt-in with the panel through the same
|
||||
// PUT /api/tools/toolsets/{name} the Toolsets tab uses —
|
||||
// scoped to the profile this page shows, since the toolset
|
||||
// is per-profile config. Refresh the Toolsets cache so its
|
||||
// row repaints, and say what happened either way.
|
||||
const toolset = DESKTOP_PLUGIN_TOOLSETS[desktop.id]
|
||||
|
||||
if (!toolset) {
|
||||
return
|
||||
}
|
||||
|
||||
try {
|
||||
await setToolsetEnabled(toolset, on, profile)
|
||||
void queryClient.invalidateQueries({ queryKey: TOOLSETS_QUERY_KEY })
|
||||
notify({
|
||||
kind: 'success',
|
||||
message: on ? p.toolsetOn(pkg.name, scopeLabel) : p.toolsetOff(pkg.name, scopeLabel)
|
||||
})
|
||||
} catch (err) {
|
||||
notifyError(err, p.toolsetToggleFailed(pkg.name))
|
||||
}
|
||||
})()
|
||||
void toggleDesktop(desktop.id, on)
|
||||
}}
|
||||
/>
|
||||
) : pkg.desktopMissing ? (
|
||||
|
||||
@@ -2793,7 +2793,8 @@ export const deOverrides = {
|
||||
toggleFailed: name => `${name} konnte nicht umgeschaltet werden`,
|
||||
toolsetOn: (name: string, profile: string) => `${name}-Agent-Tools für ${profile} aktiviert`,
|
||||
toolsetOff: (name: string, profile: string) => `${name}-Agent-Tools für ${profile} deaktiviert`,
|
||||
toolsetToggleFailed: (name: string) => `Die ${name}-Agent-Tools konnten nicht umgeschaltet werden`,
|
||||
toolsetToggleFailed: (name: string) =>
|
||||
`Die ${name}-Agent-Tools konnten nicht umgeschaltet werden; das Desktop-Panel bleibt unverändert`,
|
||||
legacyBackend:
|
||||
'Dieses Backend ist älter als schlüsseladressierte Plugin-Schalter — aktualisieren Sie Hermes, um es hier zu verwalten.',
|
||||
portableBadge: 'tragbar',
|
||||
|
||||
@@ -2489,7 +2489,8 @@ export const en: Translations = {
|
||||
toggleFailed: (name: string) => `Could not toggle ${name}`,
|
||||
toolsetOn: (name: string, profile: string) => `${name} agent tools enabled for ${profile}`,
|
||||
toolsetOff: (name: string, profile: string) => `${name} agent tools disabled for ${profile}`,
|
||||
toolsetToggleFailed: (name: string) => `Could not toggle the ${name} agent tools`,
|
||||
toolsetToggleFailed: (name: string) =>
|
||||
`Could not toggle the ${name} agent tools; the Desktop panel was left unchanged`,
|
||||
legacyBackend: 'This backend predates key-addressed plugin toggles — update Hermes to manage it here.',
|
||||
portableBadge: 'portable',
|
||||
serverStates: {
|
||||
|
||||
@@ -2784,7 +2784,8 @@ export const esOverrides = {
|
||||
toggleFailed: (name: string) => `No se pudo cambiar ${name}`,
|
||||
toolsetOn: (name: string, profile: string) => `Herramientas de agente de ${name} activadas para ${profile}`,
|
||||
toolsetOff: (name: string, profile: string) => `Herramientas de agente de ${name} desactivadas para ${profile}`,
|
||||
toolsetToggleFailed: (name: string) => `No se pudieron cambiar las herramientas de agente de ${name}`,
|
||||
toolsetToggleFailed: (name: string) =>
|
||||
`No se pudieron cambiar las herramientas de agente de ${name}; el panel de Escritorio no se modificó`,
|
||||
legacyBackend:
|
||||
'Este backend es anterior a los interruptores de plugins por clave: actualiza Hermes para gestionarlo aquí.',
|
||||
portableBadge: 'portátil',
|
||||
|
||||
@@ -2802,7 +2802,8 @@ export const frOverrides = {
|
||||
toggleFailed: name => `Impossible de modifier l'état de ${name}`,
|
||||
toolsetOn: (name: string, profile: string) => `Outils agent de ${name} activés pour ${profile}`,
|
||||
toolsetOff: (name: string, profile: string) => `Outils agent de ${name} désactivés pour ${profile}`,
|
||||
toolsetToggleFailed: (name: string) => `Impossible de modifier les outils agent de ${name}`,
|
||||
toolsetToggleFailed: (name: string) =>
|
||||
`Impossible de modifier les outils agent de ${name} ; le panneau Desktop reste inchangé`,
|
||||
legacyBackend: 'Ce backend est trop ancien pour gérer les plugins depuis cet écran ; mettez Hermes à jour.',
|
||||
portableBadge: 'portable',
|
||||
serverStates: {
|
||||
|
||||
Reference in New Issue
Block a user