refactor(file-manager): replace mutating addAllToSet/removeAllFromSet with pure addAll/removeAll

The old helpers mutated the Set passed in, but the type signature
(`Set<string>`) did not communicate that — only the JSDoc did. Every
existing caller already cloned `prev` before calling, so the migration
to pure `(prev, paths) => new Set` variants is zero-cost and removes a
foot-gun. `toggleSubtreeOnSet` and `computeRowClickSelection` now
compose the pure helpers directly instead of clone-then-mutate.

Co-authored-by: Cursor <cursoragent@cursor.com>

frontend/src/components/shared/file-manager/file-manager-utils.test.ts

@@ -4,7 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import type { FileManagerInternalNode, FileManagerRootGroup, FileNode } from './file-manager-types';
 
 import {
-    addAllToSet,
+    addAll,
     buildFileManagerGridTemplate,
     buildFileManagerTree,
     clamp,
@@ -27,7 +27,7 @@ import {
     isEverySelected,
     normalizeRootGroups,
     pluralizeItemsEnglish,
-    removeAllFromSet,
+    removeAll,
     resolveSelectionModifier,
     sortFileManagerTree,
     toggleSubtreeOnSet,
@@ -491,47 +491,59 @@ describe('pluralizeItemsEnglish', () => {
     });
 });
 
-describe('addAllToSet', () => {
-    it('adds every path from the iterable into the set', () => {
-        const set = new Set(['a']);
+describe('addAll', () => {
+    it('returns a new set containing every path from prev plus the iterable', () => {
+        const prev = new Set(['a']);
+        const next = addAll(prev, ['b', 'c']);
 
-        addAllToSet(set, ['b', 'c']);
-
-        expect([...set].sort()).toEqual(['a', 'b', 'c']);
+        expect([...next].sort()).toEqual(['a', 'b', 'c']);
     });
 
-    it('is a no-op for an empty iterable', () => {
-        const set = new Set(['a']);
+    it('does not mutate the input set', () => {
+        const prev = new Set(['a']);
+        const next = addAll(prev, ['b', 'c']);
 
-        addAllToSet(set, []);
+        expect(next).not.toBe(prev);
+        expect([...prev]).toEqual(['a']);
+    });
 
-        expect([...set]).toEqual(['a']);
+    it('returns a freshly cloned set even for an empty iterable', () => {
+        const prev = new Set(['a']);
+        const next = addAll(prev, []);
+
+        expect(next).not.toBe(prev);
+        expect([...next]).toEqual(['a']);
     });
 
     it('accepts another Set as the source', () => {
-        const set = new Set<string>();
+        const next = addAll(new Set<string>(), new Set(['x', 'y']));
 
-        addAllToSet(set, new Set(['x', 'y']));
-
-        expect([...set].sort()).toEqual(['x', 'y']);
+        expect([...next].sort()).toEqual(['x', 'y']);
     });
 });
 
-describe('removeAllFromSet', () => {
-    it('removes every present path and ignores missing ones', () => {
-        const set = new Set(['a', 'b', 'c']);
+describe('removeAll', () => {
+    it('returns a new set without the requested paths and ignores missing ones', () => {
+        const prev = new Set(['a', 'b', 'c']);
+        const next = removeAll(prev, ['b', 'missing']);
 
-        removeAllFromSet(set, ['b', 'missing']);
-
-        expect([...set].sort()).toEqual(['a', 'c']);
+        expect([...next].sort()).toEqual(['a', 'c']);
     });
 
-    it('is a no-op for an empty iterable', () => {
-        const set = new Set(['a']);
+    it('does not mutate the input set', () => {
+        const prev = new Set(['a', 'b']);
+        const next = removeAll(prev, ['b']);
 
-        removeAllFromSet(set, []);
+        expect(next).not.toBe(prev);
+        expect([...prev].sort()).toEqual(['a', 'b']);
+    });
 
-        expect([...set]).toEqual(['a']);
+    it('returns a freshly cloned set even for an empty iterable', () => {
+        const prev = new Set(['a']);
+        const next = removeAll(prev, []);
+
+        expect(next).not.toBe(prev);
+        expect([...next]).toEqual(['a']);
     });
 });
 
@@ -1060,11 +1072,11 @@ describe('computeRowClickSelection — purity', () => {
     });
 
     it('never mutates the input prev Set on a range click that expands directories', () => {
-        // Defensive: the additive range branch builds `next` via `addAllToSet`
-        // on a clone of `prev`. If the clone step ever regressed, every
-        // directory expansion would silently corrupt React state. Shaped after
-        // the user's three-folder scenario so a regression here always lights
-        // up a real-world failure mode.
+        // Defensive: the additive range branch funnels every expansion through
+        // `addAll`, which clones internally. If that contract ever regressed,
+        // every directory expansion would silently corrupt React state.
+        // Shaped after the user's three-folder scenario so a regression here
+        // always lights up a real-world failure mode.
         const prev = new Set(['f1', 'f1/a', 'f1/b']);
         const snapshot = [...prev];
 

frontend/src/components/shared/file-manager/file-manager-utils.ts

@@ -672,24 +672,33 @@ export const computeSelectionTotalBytes = (
 };
 
 /**
- * Mutates `set` in place by adding every path from `paths`. Designed for "build a
- * fresh `next` from `prev`, then add a batch" patterns inside `setState` updaters
- * — never call on a state-owned Set directly.
+ * Returns a freshly cloned Set with every path from `paths` added. Pure —
+ * safe to call on a state-owned Set inside a `setState` updater because the
+ * input is never mutated. Empty `paths` still produces a fresh clone.
  */
-export const addAllToSet = (set: Set<string>, paths: Iterable<string>): void => {
+export const addAll = (prev: ReadonlySet<string>, paths: Iterable<string>): Set<string> => {
+    const next = new Set(prev);
+
     for (const path of paths) {
-        set.add(path);
+        next.add(path);
     }
+
+    return next;
 };
 
 /**
- * Mutates `set` in place by removing every path from `paths`. Same locality
- * contract as `addAllToSet`: only meaningful on a freshly cloned Set.
+ * Returns a freshly cloned Set with every path from `paths` removed. Same
+ * purity contract as `addAll`: input is never mutated, missing paths are
+ * silently ignored.
  */
-export const removeAllFromSet = (set: Set<string>, paths: Iterable<string>): void => {
+export const removeAll = (prev: ReadonlySet<string>, paths: Iterable<string>): Set<string> => {
+    const next = new Set(prev);
+
     for (const path of paths) {
-        set.delete(path);
+        next.delete(path);
     }
+
+    return next;
 };
 
 /**
@@ -731,8 +740,6 @@ export const toggleSubtreeOnSet = (
     paths: readonly string[],
     rootPath?: string,
 ): Set<string> => {
-    const next = new Set(prev);
-
     let allSelected: boolean;
 
     if (rootPath !== undefined && paths.length > 1) {
@@ -743,22 +750,16 @@ export const toggleSubtreeOnSet = (
                 continue;
             }
 
-            if (!next.has(p)) {
+            if (!prev.has(p)) {
                 allSelected = false;
                 break;
             }
         }
     } else {
-        allSelected = isEverySelected(paths, next);
+        allSelected = isEverySelected(paths, prev);
     }
 
-    if (allSelected) {
-        removeAllFromSet(next, paths);
-    } else {
-        addAllToSet(next, paths);
-    }
-
-    return next;
+    return allSelected ? removeAll(prev, paths) : addAll(prev, paths);
 };
 
 interface ComputeRowClickSelectionArgs {
@@ -881,15 +882,8 @@ export const computeRowClickSelection = ({
     // unchecked because its descendants stayed out of the selection.
     const expandDir = (p: string): readonly string[] => dirSubtreePaths?.get(p) ?? [p];
 
-    const addRangeToPrev = (paths: Iterable<string>): Set<string> => {
-        const next = new Set(prev);
-
-        for (const p of paths) {
-            addAllToSet(next, expandDir(p));
-        }
-
-        return next;
-    };
+    const addRangeToPrev = (paths: Iterable<string>): Set<string> =>
+        addAll(prev, Array.from(paths).flatMap((p) => [...expandDir(p)]));
 
     const buildAdditiveFallback = (): Set<string> => addRangeToPrev(hasSubtree && subtreePaths ? subtreePaths : [path]);