diff --git a/src/renderer/store/slices/teamSlice.ts b/src/renderer/store/slices/teamSlice.ts index dc41b47d..b9f359ac 100644 --- a/src/renderer/store/slices/teamSlice.ts +++ b/src/renderer/store/slices/teamSlice.ts @@ -111,6 +111,7 @@ import { collectTeamScopedVisibleLoadingResets, } from '../team/teamScopedStateCleanup'; import { structurallyShareTeamSnapshot } from '../team/teamSnapshotStructuralSharing'; +import { parseToolApprovalSettings } from '../team/teamToolApprovalSettings'; import { noteTeamRefreshFanout } from '../teamRefreshFanoutDiagnostics'; import { getWorktreeNavigationState } from '../utils/stateResetHelpers'; @@ -2272,39 +2273,6 @@ function saveLaunchParams(teamName: string, params: TeamLaunchParams): void { const TOOL_APPROVAL_PREFIX = 'team:toolApprovalSettings:'; -function parseToolApprovalSettings(raw: string | null): ToolApprovalSettings { - if (!raw) return DEFAULT_TOOL_APPROVAL_SETTINGS; - try { - const parsed = JSON.parse(raw) as Record; - const d = DEFAULT_TOOL_APPROVAL_SETTINGS; - return { - autoAllowAll: typeof parsed.autoAllowAll === 'boolean' ? parsed.autoAllowAll : d.autoAllowAll, - autoAllowFileEdits: - typeof parsed.autoAllowFileEdits === 'boolean' - ? parsed.autoAllowFileEdits - : d.autoAllowFileEdits, - autoAllowSafeBash: - typeof parsed.autoAllowSafeBash === 'boolean' - ? parsed.autoAllowSafeBash - : d.autoAllowSafeBash, - timeoutAction: - typeof parsed.timeoutAction === 'string' && - ['allow', 'deny', 'wait'].includes(parsed.timeoutAction) - ? (parsed.timeoutAction as ToolApprovalSettings['timeoutAction']) - : d.timeoutAction, - timeoutSeconds: - typeof parsed.timeoutSeconds === 'number' && - Number.isFinite(parsed.timeoutSeconds) && - parsed.timeoutSeconds >= 5 && - parsed.timeoutSeconds <= 300 - ? parsed.timeoutSeconds - : d.timeoutSeconds, - }; - } catch { - return DEFAULT_TOOL_APPROVAL_SETTINGS; - } -} - function loadToolApprovalSettingsForTeam(teamName: string): ToolApprovalSettings { return parseToolApprovalSettings(localStorage.getItem(TOOL_APPROVAL_PREFIX + teamName)); } diff --git a/src/renderer/store/team/teamToolApprovalSettings.ts b/src/renderer/store/team/teamToolApprovalSettings.ts new file mode 100644 index 00000000..8edadb0c --- /dev/null +++ b/src/renderer/store/team/teamToolApprovalSettings.ts @@ -0,0 +1,42 @@ +import { DEFAULT_TOOL_APPROVAL_SETTINGS } from '@shared/types/team'; + +import type { ToolApprovalSettings } from '@shared/types'; + +const VALID_TIMEOUT_ACTIONS: ReadonlySet = new Set([ + 'allow', + 'deny', + 'wait', +]); + +export function parseToolApprovalSettings(raw: string | null): ToolApprovalSettings { + if (!raw) return DEFAULT_TOOL_APPROVAL_SETTINGS; + try { + const parsed = JSON.parse(raw) as Record; + const d = DEFAULT_TOOL_APPROVAL_SETTINGS; + return { + autoAllowAll: typeof parsed.autoAllowAll === 'boolean' ? parsed.autoAllowAll : d.autoAllowAll, + autoAllowFileEdits: + typeof parsed.autoAllowFileEdits === 'boolean' + ? parsed.autoAllowFileEdits + : d.autoAllowFileEdits, + autoAllowSafeBash: + typeof parsed.autoAllowSafeBash === 'boolean' + ? parsed.autoAllowSafeBash + : d.autoAllowSafeBash, + timeoutAction: + typeof parsed.timeoutAction === 'string' && + VALID_TIMEOUT_ACTIONS.has(parsed.timeoutAction as ToolApprovalSettings['timeoutAction']) + ? (parsed.timeoutAction as ToolApprovalSettings['timeoutAction']) + : d.timeoutAction, + timeoutSeconds: + typeof parsed.timeoutSeconds === 'number' && + Number.isFinite(parsed.timeoutSeconds) && + parsed.timeoutSeconds >= 5 && + parsed.timeoutSeconds <= 300 + ? parsed.timeoutSeconds + : d.timeoutSeconds, + }; + } catch { + return DEFAULT_TOOL_APPROVAL_SETTINGS; + } +} diff --git a/test/renderer/store/teamToolApprovalSettings.test.ts b/test/renderer/store/teamToolApprovalSettings.test.ts new file mode 100644 index 00000000..8d7412a9 --- /dev/null +++ b/test/renderer/store/teamToolApprovalSettings.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest'; + +import { parseToolApprovalSettings } from '../../../src/renderer/store/team/teamToolApprovalSettings'; +import { DEFAULT_TOOL_APPROVAL_SETTINGS } from '../../../src/shared/types/team'; + +describe('teamToolApprovalSettings', () => { + it('returns defaults for missing or invalid JSON', () => { + expect(parseToolApprovalSettings(null)).toBe(DEFAULT_TOOL_APPROVAL_SETTINGS); + expect(parseToolApprovalSettings('')).toBe(DEFAULT_TOOL_APPROVAL_SETTINGS); + expect(parseToolApprovalSettings('{not json')).toBe(DEFAULT_TOOL_APPROVAL_SETTINGS); + }); + + it('parses valid complete settings', () => { + expect( + parseToolApprovalSettings( + JSON.stringify({ + autoAllowAll: true, + autoAllowFileEdits: true, + autoAllowSafeBash: true, + timeoutAction: 'allow', + timeoutSeconds: 120, + }) + ) + ).toEqual({ + autoAllowAll: true, + autoAllowFileEdits: true, + autoAllowSafeBash: true, + timeoutAction: 'allow', + timeoutSeconds: 120, + }); + }); + + it('falls back per field when values have invalid types', () => { + expect( + parseToolApprovalSettings( + JSON.stringify({ + autoAllowAll: 'yes', + autoAllowFileEdits: true, + autoAllowSafeBash: 1, + timeoutAction: 'maybe', + timeoutSeconds: '60', + }) + ) + ).toEqual({ + ...DEFAULT_TOOL_APPROVAL_SETTINGS, + autoAllowFileEdits: true, + }); + }); + + it('accepts timeout actions allow, deny, and wait', () => { + expect(parseToolApprovalSettings(JSON.stringify({ timeoutAction: 'allow' })).timeoutAction).toBe( + 'allow' + ); + expect(parseToolApprovalSettings(JSON.stringify({ timeoutAction: 'deny' })).timeoutAction).toBe( + 'deny' + ); + expect(parseToolApprovalSettings(JSON.stringify({ timeoutAction: 'wait' })).timeoutAction).toBe( + 'wait' + ); + }); + + it('accepts timeout seconds at inclusive boundaries', () => { + expect(parseToolApprovalSettings(JSON.stringify({ timeoutSeconds: 5 })).timeoutSeconds).toBe(5); + expect(parseToolApprovalSettings(JSON.stringify({ timeoutSeconds: 300 })).timeoutSeconds).toBe( + 300 + ); + }); + + it('rejects timeout seconds outside allowed boundaries or non-finite values', () => { + expect(parseToolApprovalSettings(JSON.stringify({ timeoutSeconds: 4 })).timeoutSeconds).toBe( + DEFAULT_TOOL_APPROVAL_SETTINGS.timeoutSeconds + ); + expect(parseToolApprovalSettings(JSON.stringify({ timeoutSeconds: 301 })).timeoutSeconds).toBe( + DEFAULT_TOOL_APPROVAL_SETTINGS.timeoutSeconds + ); + expect( + parseToolApprovalSettings(JSON.stringify({ timeoutSeconds: Number.POSITIVE_INFINITY })) + .timeoutSeconds + ).toBe(DEFAULT_TOOL_APPROVAL_SETTINGS.timeoutSeconds); + }); +});