From 644214a8d47a2cc28f9d049c1e4e6d2debefd03a Mon Sep 17 00:00:00 2001 From: "buf0-bot[bot]" <252831055+buf0-bot[bot]@users.noreply.github.com> Date: Fri, 29 May 2026 16:59:20 -0700 Subject: [PATCH] fix: pr-bug-scan validated finding from #2972 Fix Settings > Agents availability updates so rapid queued writes preserve all requested disabled agents and repeated requests remain idempotent. --- .../components/settings/AgentsPane.test.tsx | 146 ++++++++++++++++-- .../src/components/settings/AgentsPane.tsx | 70 +++++++-- 2 files changed, 190 insertions(+), 26 deletions(-) diff --git a/src/renderer/src/components/settings/AgentsPane.test.tsx b/src/renderer/src/components/settings/AgentsPane.test.tsx index 22c3328ce5a..8d146ad11f5 100644 --- a/src/renderer/src/components/settings/AgentsPane.test.tsx +++ b/src/renderer/src/components/settings/AgentsPane.test.tsx @@ -1,3 +1,5 @@ +/* eslint-disable max-lines -- Why: Agents pane settings interactions share + store-backed queue fixtures that are easier to audit beside the UI helper coverage. */ import React from 'react' import { renderToStaticMarkup } from 'react-dom/server' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -12,7 +14,8 @@ import { AgentStatusHooksSetting, AgentsPane, AGENTS_PANE_SEARCH_ENTRIES, - buildAgentEnabledSettingsUpdate + buildAgentAvailabilitySettingsUpdate, + createAgentAvailabilityUpdateQueue } from './AgentsPane' import { matchesSettingsSearch } from './settings-search' @@ -35,6 +38,24 @@ type ReactElementLike = { props: Record } +type Deferred = { + promise: Promise + resolve: () => void +} + +function createDeferred(): Deferred { + let resolve!: () => void + const promise = new Promise((next) => { + resolve = next + }) + return { promise, resolve } +} + +async function flushPromiseQueue(): Promise { + await Promise.resolve() + await Promise.resolve() +} + function renderPane( settings: GlobalSettings, props: Partial> = {} @@ -212,11 +233,11 @@ describe('AgentsPane', () => { }) it('only toggles agent availability when the segmented value changes', () => { - const onToggleEnabled = vi.fn() + const onSetEnabled = vi.fn() const control = AgentAvailabilityControl({ label: 'Claude', isEnabled: true, - onToggleEnabled + onSetEnabled }) const props = control.props as { value: 'enabled' | 'disabled' @@ -228,20 +249,21 @@ describe('AgentsPane', () => { expect(props.ariaLabel).toBe('Claude availability') props.onChange('enabled') - expect(onToggleEnabled).not.toHaveBeenCalled() + expect(onSetEnabled).not.toHaveBeenCalled() props.onChange('disabled') - expect(onToggleEnabled).toHaveBeenCalledTimes(1) + expect(onSetEnabled).toHaveBeenCalledWith(false) }) it('clears the default agent when disabling that agent', () => { expect( - buildAgentEnabledSettingsUpdate( + buildAgentAvailabilitySettingsUpdate( { defaultTuiAgent: 'claude', disabledTuiAgents: [] }, - 'claude' + 'claude', + false ) ).toEqual({ disabledTuiAgents: ['claude'], @@ -251,12 +273,13 @@ describe('AgentsPane', () => { it('keeps the default setting untouched when re-enabling an agent', () => { expect( - buildAgentEnabledSettingsUpdate( + buildAgentAvailabilitySettingsUpdate( { defaultTuiAgent: null, disabledTuiAgents: ['claude'] }, - 'claude' + 'claude', + true ) ).toEqual({ disabledTuiAgents: [] @@ -267,4 +290,109 @@ describe('AgentsPane', () => { expect(matchesSettingsSearch('wsl', AGENTS_PANE_SEARCH_ENTRIES)).toBe(true) expect(matchesSettingsSearch('windows', AGENTS_PANE_SEARCH_ENTRIES)).toBe(true) }) + + it('serializes rapid availability writes against the latest settings snapshot', async () => { + const queueAvailabilityUpdate = createAgentAvailabilityUpdateQueue() + const settings: GlobalSettings = { + ...getDefaultSettings('/tmp'), + defaultTuiAgent: null, + disabledTuiAgents: [] + } + const writes: Deferred[] = [] + const updates: Partial[] = [] + + useAppStore.setState({ settings }) + const updateSettings = vi.fn((update: Partial) => { + updates.push(update) + const nextSettings = { + ...(useAppStore.getState().settings ?? settings), + ...update + } + const write = createDeferred() + writes.push(write) + return write.promise.then(() => { + useAppStore.setState({ settings: nextSettings }) + }) + }) + + const firstWrite = queueAvailabilityUpdate({ + getSettings: () => useAppStore.getState().settings, + fallbackSettings: settings, + updateSettings, + agentId: 'claude', + enabled: false + }) + const secondWrite = queueAvailabilityUpdate({ + getSettings: () => useAppStore.getState().settings, + fallbackSettings: settings, + updateSettings, + agentId: 'codex', + enabled: false + }) + + await flushPromiseQueue() + expect(updateSettings).toHaveBeenCalledTimes(1) + expect(updates[0]).toMatchObject({ disabledTuiAgents: ['claude'] }) + + writes[0].resolve() + await firstWrite + await flushPromiseQueue() + + expect(updateSettings).toHaveBeenCalledTimes(2) + expect(updates[1]).toMatchObject({ disabledTuiAgents: ['claude', 'codex'] }) + + writes[1].resolve() + await secondWrite + }) + + it('keeps repeated queued availability requests idempotent', async () => { + const queueAvailabilityUpdate = createAgentAvailabilityUpdateQueue() + const settings: GlobalSettings = { + ...getDefaultSettings('/tmp'), + defaultTuiAgent: null, + disabledTuiAgents: [] + } + const writes: Deferred[] = [] + const updates: Partial[] = [] + + useAppStore.setState({ settings }) + const updateSettings = vi.fn((update: Partial) => { + updates.push(update) + const nextSettings = { + ...(useAppStore.getState().settings ?? settings), + ...update + } + const write = createDeferred() + writes.push(write) + return write.promise.then(() => { + useAppStore.setState({ settings: nextSettings }) + }) + }) + + const firstWrite = queueAvailabilityUpdate({ + getSettings: () => useAppStore.getState().settings, + fallbackSettings: settings, + updateSettings, + agentId: 'claude', + enabled: false + }) + const secondWrite = queueAvailabilityUpdate({ + getSettings: () => useAppStore.getState().settings, + fallbackSettings: settings, + updateSettings, + agentId: 'claude', + enabled: false + }) + + await flushPromiseQueue() + writes[0].resolve() + await firstWrite + await flushPromiseQueue() + + expect(updateSettings).toHaveBeenCalledTimes(2) + expect(updates[1]).toMatchObject({ disabledTuiAgents: ['claude'] }) + + writes[1].resolve() + await secondWrite + }) }) diff --git a/src/renderer/src/components/settings/AgentsPane.tsx b/src/renderer/src/components/settings/AgentsPane.tsx index 99eb3ef9b48..0a002ab017c 100644 --- a/src/renderer/src/components/settings/AgentsPane.tsx +++ b/src/renderer/src/components/settings/AgentsPane.tsx @@ -34,6 +34,14 @@ type AgentsPaneProps = { wslCapabilitiesLoading?: boolean } +type AgentAvailabilityUpdateQueueOptions = { + getSettings: () => GlobalSettings | null | undefined + fallbackSettings: GlobalSettings + updateSettings: AgentsPaneProps['updateSettings'] + agentId: TuiAgent + enabled: boolean +} + type AgentRowProps = { agentId: TuiAgent label: string @@ -44,7 +52,7 @@ type AgentRowProps = { isDefault: boolean cmdOverride: string | undefined onSetDefault: () => void - onToggleEnabled: () => void + onSetEnabled: (enabled: boolean) => void onSaveOverride: (value: string) => void } @@ -59,29 +67,52 @@ type AgentAvailability = 'enabled' | 'disabled' type AgentAvailabilityControlProps = { label: string isEnabled: boolean - onToggleEnabled: () => void + onSetEnabled: (enabled: boolean) => void } -export function buildAgentEnabledSettingsUpdate( +export function buildAgentAvailabilitySettingsUpdate( settings: Pick, - id: TuiAgent + id: TuiAgent, + enabled: boolean ): Pick & Partial> { const latestDisabled = normalizeDisabledTuiAgents(settings.disabledTuiAgents) - const wasDisabled = latestDisabled.includes(id) - const nextDisabled = wasDisabled + const nextDisabled = enabled ? latestDisabled.filter((agent) => agent !== id) - : [...latestDisabled, id] + : latestDisabled.includes(id) + ? latestDisabled + : [...latestDisabled, id] return { disabledTuiAgents: nextDisabled, - ...(settings.defaultTuiAgent === id && !wasDisabled ? { defaultTuiAgent: null } : {}) + ...(settings.defaultTuiAgent === id && !enabled ? { defaultTuiAgent: null } : {}) } } +export function createAgentAvailabilityUpdateQueue(): ( + options: AgentAvailabilityUpdateQueueOptions +) => Promise { + let pendingUpdate: Promise = Promise.resolve() + + return ({ getSettings, fallbackSettings, updateSettings, agentId, enabled }) => { + // Why: serialize full-array replacements so each write sees the store after + // the previous IPC has reconciled, while preserving the user's requested state. + pendingUpdate = pendingUpdate + .catch(() => {}) + .then(() => + updateSettings( + buildAgentAvailabilitySettingsUpdate(getSettings() ?? fallbackSettings, agentId, enabled) + ) + ) + return pendingUpdate.then(() => undefined) + } +} + +const enqueueAgentAvailabilityUpdate = createAgentAvailabilityUpdateQueue() + export function AgentAvailabilityControl({ label, isEnabled, - onToggleEnabled + onSetEnabled }: AgentAvailabilityControlProps): React.JSX.Element { const value: AgentAvailability = isEnabled ? 'enabled' : 'disabled' @@ -90,7 +121,7 @@ export function AgentAvailabilityControl({ value={value} onChange={(next) => { if (next !== value) { - onToggleEnabled() + onSetEnabled(next === 'enabled') } }} ariaLabel={`${label} availability`} @@ -170,7 +201,7 @@ function AgentRow({ isDefault, cmdOverride, onSetDefault, - onToggleEnabled, + onSetEnabled, onSaveOverride }: AgentRowProps): React.JSX.Element { const [cmdOpen, setCmdOpen] = useState(Boolean(cmdOverride)) @@ -216,7 +247,7 @@ function AgentRow({ {isDetected && isEnabled && ( @@ -346,9 +377,14 @@ export function AgentsPane({ updateSettings({ defaultTuiAgent: id }) } - const toggleEnabled = (id: TuiAgent): void => { - const latestSettings = useAppStore.getState().settings ?? settings - updateSettings(buildAgentEnabledSettingsUpdate(latestSettings, id)) + const setAgentEnabled = (id: TuiAgent, enabled: boolean): void => { + void enqueueAgentAvailabilityUpdate({ + getSettings: () => useAppStore.getState().settings, + fallbackSettings: settings, + updateSettings, + agentId: id, + enabled + }) } const saveOverride = (id: TuiAgent, value: string): void => { @@ -474,7 +510,7 @@ export function AgentsPane({ isDefault={defaultAgent === agent.id} cmdOverride={cmdOverrides[agent.id]} onSetDefault={() => setDefault(agent.id)} - onToggleEnabled={() => toggleEnabled(agent.id)} + onSetEnabled={(enabled) => setAgentEnabled(agent.id, enabled)} onSaveOverride={(v) => saveOverride(agent.id, v)} /> ))} @@ -506,7 +542,7 @@ export function AgentsPane({ isDefault={false} cmdOverride={undefined} onSetDefault={() => {}} - onToggleEnabled={() => toggleEnabled(agent.id)} + onSetEnabled={(enabled) => setAgentEnabled(agent.id, enabled)} onSaveOverride={() => {}} /> ))}