mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 16:02:38 +00:00
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.
This commit is contained in:
@@ -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<string, unknown>
|
||||
}
|
||||
|
||||
type Deferred = {
|
||||
promise: Promise<void>
|
||||
resolve: () => void
|
||||
}
|
||||
|
||||
function createDeferred(): Deferred {
|
||||
let resolve!: () => void
|
||||
const promise = new Promise<void>((next) => {
|
||||
resolve = next
|
||||
})
|
||||
return { promise, resolve }
|
||||
}
|
||||
|
||||
async function flushPromiseQueue(): Promise<void> {
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
}
|
||||
|
||||
function renderPane(
|
||||
settings: GlobalSettings,
|
||||
props: Partial<React.ComponentProps<typeof AgentsPane>> = {}
|
||||
@@ -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<GlobalSettings>[] = []
|
||||
|
||||
useAppStore.setState({ settings })
|
||||
const updateSettings = vi.fn((update: Partial<GlobalSettings>) => {
|
||||
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<GlobalSettings>[] = []
|
||||
|
||||
useAppStore.setState({ settings })
|
||||
const updateSettings = vi.fn((update: Partial<GlobalSettings>) => {
|
||||
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
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<GlobalSettings, 'defaultTuiAgent' | 'disabledTuiAgents'>,
|
||||
id: TuiAgent
|
||||
id: TuiAgent,
|
||||
enabled: boolean
|
||||
): Pick<GlobalSettings, 'disabledTuiAgents'> & Partial<Pick<GlobalSettings, 'defaultTuiAgent'>> {
|
||||
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<void> {
|
||||
let pendingUpdate: Promise<unknown> = 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({
|
||||
<AgentAvailabilityControl
|
||||
label={label}
|
||||
isEnabled={isEnabled}
|
||||
onToggleEnabled={onToggleEnabled}
|
||||
onSetEnabled={onSetEnabled}
|
||||
/>
|
||||
|
||||
{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={() => {}}
|
||||
/>
|
||||
))}
|
||||
|
||||
Reference in New Issue
Block a user