refactor(team): extract tool approval parser

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<string, unknown>;
-    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));
 }

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<ToolApprovalSettings['timeoutAction']> = 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<string, unknown>;
+    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;
+  }
+}

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);
+  });
+});