From c48a72b631b0bdde8bd488979cb3ffa76e89e45e Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 16 Sep 2026 02:09:56 -0400 Subject: [PATCH] feat(settings): offer one stateless Enable on all computers button The row above the list is now just that button. It appears when a paired server is reachable, new enough and off, acts on exactly those plus this computer, and disappears when there is nothing left to do. What it offers is read off the rows each render, so it cannot disagree with them. Deletes the standing auto-enable consent with it: the persisted flag, the code that armed and cleared it, the per-host memory of which switches the user had touched, and the line that promised future computers would turn themselves on. A preference that acts on hosts the user never sees is worse than a button they press when they mean it. --- .../settings/SessionHistoryServerRow.tsx | 5 +- .../SessionHistorySettingsPane.test.tsx | 92 ++++++++---------- .../settings/SessionHistorySettingsPane.tsx | 88 ++++------------- .../session-search-computer-rollup.test.ts | 36 +------ .../session-search-computer-rollup.ts | 56 ----------- .../use-session-search-auto-enable.ts | Bin 2312 -> 0 bytes src/renderer/src/i18n/locales/en.json | 8 +- src/shared/global-settings-types.ts | 7 -- 8 files changed, 62 insertions(+), 230 deletions(-) delete mode 100644 src/renderer/src/components/settings/use-session-search-auto-enable.ts diff --git a/src/renderer/src/components/settings/SessionHistoryServerRow.tsx b/src/renderer/src/components/settings/SessionHistoryServerRow.tsx index daf93e720d2..9cd1c791b5d 100644 --- a/src/renderer/src/components/settings/SessionHistoryServerRow.tsx +++ b/src/renderer/src/components/settings/SessionHistoryServerRow.tsx @@ -25,8 +25,7 @@ export function SessionHistoryServerRow({ details, refresh = 0, onError, - onStateChange, - onUserToggle + onStateChange }: { environment: PublicKnownRuntimeEnvironment details: RuntimeHostDetails | undefined @@ -35,7 +34,6 @@ export function SessionHistoryServerRow({ onError: (message: string | null) => void /** Lets the pane count and order computers it does not itself poll. */ onStateChange?: (environmentId: string, state: SessionSearchComputerState) => void - onUserToggle?: (environmentId: string, enabled: boolean) => void }): React.JSX.Element { const hostId = toRuntimeExecutionHostId(environment.id) const mounted = useMountedRef() @@ -93,7 +91,6 @@ export function SessionHistoryServerRow({ } function toggle(): Promise { - onUserToggle?.(environment.id, !enabled) return setEnabled(!enabled) } diff --git a/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx b/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx index 270b651e24d..909b2839b11 100644 --- a/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx +++ b/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx @@ -59,16 +59,14 @@ function pane( enabled = false, confirm = vi.fn().mockResolvedValue(true), save = vi.fn().mockResolvedValue(undefined), - historyDays: number | null = null, - autoEnableNewComputers = false + historyDays: number | null = null ) { return render( @@ -110,7 +108,7 @@ function statusByHost(): void { return answer }) } -const summaryLine = (): string => screen.getByText(/computers/).textContent ?? '' +const enableAllButton = () => screen.queryByRole('button', { name: 'Enable on all computers' }) async function openAdvanced(): Promise { await act(async () => { fireEvent.click(screen.getByRole('button', { name: /Advanced/ })) @@ -359,7 +357,7 @@ it('offers only this computer to a paired client, with no server rows', async () expect(screen.getAllByRole('switch')).toHaveLength(1) expect(screen.getByRole('switch')).toBeDisabled() expect(screen.queryByRole('status')).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Turn on all' })).not.toBeInTheDocument() + expect(enableAllButton()).not.toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Open' })).not.toBeInTheDocument() expect(mocks.status).not.toHaveBeenCalled() }) @@ -391,57 +389,65 @@ it('leaves a lone computer to its own switch, with no roll-up above it', async ( pane(true) await act(async () => {}) expect(screen.getAllByRole('switch')).toHaveLength(1) - expect(screen.queryByText(/of 1 computers/)).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Turn on all' })).not.toBeInTheDocument() + expect(enableAllButton()).not.toBeInTheDocument() expect(screen.queryByText('This computer')).not.toBeInTheDocument() expect(screen.queryByText('Orca remote servers')).not.toBeInTheDocument() }) -it('counts every computer in one line, leaving out the segments worth zero', async () => { +it('offers the button only while a paired server is reachable and off', async () => { mixedFleet() pane(true) await act(async () => {}) - expect(summaryLine()).toBe('On 2 of 5 computers · 1 offline · 1 need an update') - mocks.environments = [{ id: 'off', name: 'gpu-a' }] - mocks.details = { off: CONNECTED_DETAILS } - mocks.statusByHost = { local: current, 'runtime:off': off } + expect(enableAllButton()).toBeInTheDocument() + + // gpu-a was the only eligible one; with it on, the offline and too-old rows leave nothing to do. + mocks.statusByHost = { ...mocks.statusByHost, 'runtime:off': current } statusByHost() cleanup() pane(true) await act(async () => {}) - expect(summaryLine()).toBe('On 1 of 2 computers') + expect(enableAllButton()).not.toBeInTheDocument() }) -it('turns on every reachable computer and skips the ones it cannot', async () => { +it('does not offer the button for a server whose state is still unknown', async () => { + mocks.environments = [{ id: 'a', name: 'gpu-a' }] + mocks.details = {} + mocks.statusByHost = { local: current } + statusByHost() + pane(true) + await act(async () => {}) + expect(enableAllButton()).not.toBeInTheDocument() +}) + +it('enables every reachable server and skips the ones it cannot', async () => { mixedFleet() const confirm = vi.fn().mockResolvedValue(true) - const save = vi.fn().mockResolvedValue(undefined) - pane(true, confirm, save) + pane(true, confirm) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) + fireEvent.click(screen.getByRole('button', { name: 'Enable on all computers' })) }) expect(confirm).not.toHaveBeenCalled() + // linux 1 is offline and m4 air is too old, so neither is asked; build-01 is already on. expect(mocks.setEnabled.mock.calls.map((call) => call[0])).toEqual(['runtime:off']) - expect(save).toHaveBeenCalledWith({ aiVaultSearchAutoEnableNewComputers: true }) }) -it('turns this computer on as part of turning them all on', async () => { +it('enables this computer as part of enabling them all', async () => { mocks.environments = [{ id: 'off', name: 'gpu-a' }] mocks.details = { off: CONNECTED_DETAILS } mocks.statusByHost = { local: off, 'runtime:off': off } statusByHost() const save = vi.fn().mockResolvedValue(undefined) - pane(false, vi.fn().mockResolvedValue(true), save) + pane(false, undefined, save) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) + fireEvent.click(screen.getByRole('button', { name: 'Enable on all computers' })) }) expect(save).toHaveBeenCalledWith({ aiVaultSearch: { enabled: true, historyDays: null } }) expect(mocks.setEnabled).toHaveBeenCalledWith('runtime:off', true) }) -it('keeps going after a host refuses, and withholds the standing consent', async () => { +it('keeps going after a host refuses, and names the one that did', async () => { mocks.environments = [ { id: 'a', name: 'gpu-a' }, { id: 'b', name: 'gpu-b' } @@ -450,53 +456,31 @@ it('keeps going after a host refuses, and withholds the standing consent', async mocks.statusByHost = { local: current, 'runtime:a': off, 'runtime:b': off } statusByHost() mocks.setEnabled.mockRejectedValueOnce(new Error('relay down')).mockResolvedValue(current) - const save = vi.fn().mockResolvedValue(undefined) - pane(true, vi.fn().mockResolvedValue(true), save) + pane(true) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) + fireEvent.click(screen.getByRole('button', { name: 'Enable on all computers' })) }) expect(mocks.setEnabled.mock.calls.map((call) => call[0])).toEqual(['runtime:a', 'runtime:b']) expect(screen.getByRole('alert')).toHaveTextContent('Could not change session search on gpu-a') - expect(save).not.toHaveBeenCalledWith({ aiVaultSearchAutoEnableNewComputers: true }) }) -it('hides the button and says so once nothing is left to turn on', async () => { - mocks.environments = [ - { id: 'a', name: 'gpu-a' }, - { id: 'gone', name: 'linux 1' } - ] - mocks.details = { a: CONNECTED_DETAILS, gone: OFFLINE_DETAILS } - mocks.statusByHost = { local: current, 'runtime:a': current } - statusByHost() - pane(true, undefined, undefined, null, true) - await act(async () => {}) - expect(screen.queryByRole('button', { name: 'Turn on all' })).not.toBeInTheDocument() - expect(summaryLine()).toBe('On 2 of 3 computers · 1 offline New computers turn on when they can.') -}) - -it('turns on a newly reachable server while the standing consent holds', async () => { - mocks.environments = [{ id: 'a', name: 'gpu-a' }] - mocks.details = { a: CONNECTED_DETAILS } - mocks.statusByHost = { local: current, 'runtime:a': off } - statusByHost() - pane(true, undefined, undefined, null, true) - await act(async () => {}) - expect(mocks.setEnabled).toHaveBeenCalledWith('runtime:a', true) -}) - -it('drops the standing consent when a server is turned off by hand', async () => { +it('remembers nothing after a server is turned back off by hand', async () => { mocks.environments = [{ id: 'a', name: 'gpu-a' }] mocks.details = { a: CONNECTED_DETAILS } mocks.statusByHost = { local: current, 'runtime:a': current } statusByHost() const save = vi.fn().mockResolvedValue(undefined) - pane(true, vi.fn().mockResolvedValue(true), save, null, true) + pane(true, undefined, save) await act(async () => {}) + expect(enableAllButton()).not.toBeInTheDocument() + mocks.setEnabled.mockResolvedValue(off) await act(async () => { fireEvent.click(screen.getByRole('switch', { name: 'Search sessions on gpu-a' })) }) - expect(save).toHaveBeenCalledWith({ aiVaultSearchAutoEnableNewComputers: false }) + // The row went off, so the offer comes straight back; no preference was written either way. + expect(enableAllButton()).toBeInTheDocument() + expect(save).not.toHaveBeenCalled() }) it('folds the list past six computers and orders it by what the user can act on', async () => { diff --git a/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx b/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx index b2ec893f0a0..cee4abc22c0 100644 --- a/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx +++ b/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx @@ -22,8 +22,6 @@ import { SessionSearchComputerList } from './SessionSearchComputerList' import { isTurnOnableSessionSearchState, orderSessionSearchServers, - sessionSearchSummarySentence, - summarizeSessionSearchComputers, type SessionSearchComputerEntry, type SessionSearchComputerState } from './session-search-computer-rollup' @@ -33,7 +31,6 @@ import { sessionSearchStatusDetails, sessionSearchStatusMessage } from './session-history-status-copy' -import { useSessionSearchAutoEnable } from './use-session-search-auto-enable' import { useSessionSearchStatus } from './use-session-search-status' import { useRuntimeEnvironmentCatalog } from './use-runtime-environment-catalog' @@ -45,7 +42,6 @@ export function SessionHistorySettingsPane({ updateSettings: (updates: Partial) => Promise }): React.JSX.Element { const policy = resolveAiVaultSearchSettings(settings) - const autoEnableNewComputers = settings.aiVaultSearchAutoEnableNewComputers === true const isWebClient = isWebClientLocation() const closeSettingsPage = useAppStore((state) => state.closeSettingsPage) const showAiVaultSearch = useAppStore((state) => state.showAiVaultSearch) @@ -53,9 +49,6 @@ export function SessionHistorySettingsPane({ const [error, setError] = useState(null) const [refresh, setRefresh] = useState(0) const [serverStates, setServerStates] = useState>({}) - const [userToggledServers, setUserToggledServers] = useState>( - () => new Set() - ) const { environments, detailsByEnvironmentId } = useRuntimeEnvironmentCatalog() const localRead = useSessionSearchStatus({ executionHostId: LOCAL_EXECUTION_HOST_ID, @@ -82,18 +75,11 @@ export function SessionHistorySettingsPane({ state: serverStates[environment.id] ?? 'checking', environment })) - const summary = summarizeSessionSearchComputers([localEntry, ...serverEntries]) const orderedServers = orderSessionSearchServers(serverEntries) - // Rebuilt each render on purpose: the hook keys off the host ids, not this array. - const autoEnableTargets = serverEntries - .filter( - (entry) => isTurnOnableSessionSearchState(entry.state) && !userToggledServers.has(entry.id) - ) - .map((entry) => ({ - id: entry.id, - hostId: toRuntimeExecutionHostId(entry.id), - name: entry.name - })) + // The button's whole reason to exist: a paired server this client could switch on right now. + const enableableServers = serverEntries.filter((entry) => + isTurnOnableSessionSearchState(entry.state) + ) const handleServerState = useCallback( (environmentId: string, state: SessionSearchComputerState) => { @@ -130,49 +116,26 @@ export function SessionHistorySettingsPane({ return save({ enabled: !policy.enabled }) } - /** A hand-off the user made themselves overrides the standing "turn on new computers" consent. */ - function noteServerToggledByHand(environmentId: string, enabled: boolean): void { - setUserToggledServers((current) => { - if (current.has(environmentId)) { - return current - } - const next = new Set(current) - next.add(environmentId) - return next - }) - if (!enabled && autoEnableNewComputers) { - void updateSettings({ aiVaultSearchAutoEnableNewComputers: false }) - } - } - - async function turnOnEveryComputer(): Promise { + /** Remembers nothing: what it acts on is read off the rows at the moment it is clicked. */ + async function enableOnAllComputers(): Promise { setBusy(true) setError(null) - let failed = false try { if (!policy.enabled) { try { await writePolicy({ enabled: true }) } catch { - failed = true setError(saveErrorMessage()) } } // One host at a time: a failure is that host's, and it must not stop the rest. - for (const entry of orderedServers) { - if (!isTurnOnableSessionSearchState(entry.state)) { - continue - } + for (const entry of enableableServers) { try { await window.api.aiVault.setSearchEnabled(toRuntimeExecutionHostId(entry.id), true) } catch { - failed = true setError(serverToggleErrorMessage(entry.name)) } } - if (!failed) { - await updateSettings({ aiVaultSearchAutoEnableNewComputers: true }) - } } finally { if (mounted.current) { setBusy(false) @@ -181,13 +144,6 @@ export function SessionHistorySettingsPane({ } } - useSessionSearchAutoEnable({ - active: autoEnableNewComputers && !isWebClient, - targets: autoEnableTargets, - onError: setError, - onSettled: () => setRefresh((value) => value + 1) - }) - /** False when the settings write failed or the pane went away, so the delete is skipped. */ async function turnSearchOffBeforeDelete(): Promise { try { @@ -230,23 +186,18 @@ export function SessionHistorySettingsPane({ )}

- {/* With no paired server the line and the button only restate the single switch below them. */} - {serverEntries.length === 0 ? null : ( -
-

- {sessionSearchSummarySentence(summary, autoEnableNewComputers)} -

- {summary.turnOnable > 0 ? ( - - ) : null} + {/* Nothing left to switch on means nothing to offer: each row already speaks for itself. */} + {enableableServers.length === 0 ? null : ( +
+
)} ) }))} diff --git a/src/renderer/src/components/settings/session-search-computer-rollup.test.ts b/src/renderer/src/components/settings/session-search-computer-rollup.test.ts index 66f94a1a33b..454be5e31bd 100644 --- a/src/renderer/src/components/settings/session-search-computer-rollup.test.ts +++ b/src/renderer/src/components/settings/session-search-computer-rollup.test.ts @@ -1,17 +1,10 @@ -import { expect, it, vi } from 'vitest' +import { expect, it } from 'vitest' import { isTurnOnableSessionSearchState, orderSessionSearchServers, - sessionSearchSummarySentence, - summarizeSessionSearchComputers, type SessionSearchComputerEntry } from './session-search-computer-rollup' -vi.mock('@/i18n/i18n', () => ({ - translate: (_key: string, fallback: string, args?: Record) => - fallback.replace(/{{(\w+)}}/g, (_, key: string) => String(args?.[key])) -})) - const fleet: SessionSearchComputerEntry[] = [ { id: 'local', name: 'Local Mac', state: 'on' }, { id: 'a', name: 'build-01', state: 'on' }, @@ -22,37 +15,12 @@ const fleet: SessionSearchComputerEntry[] = [ { id: 'f', name: 'probing', state: 'checking' } ] -it('counts what the user can see and what they could act on', () => { - expect(summarizeSessionSearchComputers(fleet)).toEqual({ - on: 2, - total: 7, - offline: 2, - needUpdate: 1, - turnOnable: 1 - }) -}) - it('will not offer to turn on a computer it cannot reach or that is too old', () => { expect(isTurnOnableSessionSearchState('off')).toBe(true) for (const state of ['on', 'offline', 'needs-update', 'checking'] as const) { expect(isTurnOnableSessionSearchState(state)).toBe(false) } -}) - -it('leaves a zero segment out of the sentence rather than printing it', () => { - expect(sessionSearchSummarySentence(summarizeSessionSearchComputers(fleet), false)).toBe( - 'On 2 of 7 computers · 2 offline · 1 need an update' - ) - const onlyLocal = summarizeSessionSearchComputers([fleet[0]]) - expect(sessionSearchSummarySentence(onlyLocal, false)).toBe('On 1 of 1 computers') -}) - -it('promises to keep new computers turned on only when that is the standing consent', () => { - const summary = summarizeSessionSearchComputers([fleet[0]]) - expect(sessionSearchSummarySentence(summary, true)).toBe( - 'On 1 of 1 computers New computers turn on when they can.' - ) - expect(sessionSearchSummarySentence(summary, false)).not.toContain('New computers') + expect(fleet.filter((entry) => isTurnOnableSessionSearchState(entry.state))).toHaveLength(1) }) it('orders reachable and working first, then by name inside each group', () => { diff --git a/src/renderer/src/components/settings/session-search-computer-rollup.ts b/src/renderer/src/components/settings/session-search-computer-rollup.ts index 9a9c693e3f9..2d830f8584b 100644 --- a/src/renderer/src/components/settings/session-search-computer-rollup.ts +++ b/src/renderer/src/components/settings/session-search-computer-rollup.ts @@ -1,5 +1,3 @@ -import { translate } from '@/i18n/i18n' - /** * What one computer in the pane is doing, as far as this client can tell. * @@ -14,64 +12,10 @@ export type SessionSearchComputerEntry = { state: SessionSearchComputerState } -export type SessionSearchFleetSummary = { - on: number - total: number - offline: number - needUpdate: number - /** Reachable, new enough, and still off: exactly what Turn on all would act on. */ - turnOnable: number -} - export function isTurnOnableSessionSearchState(state: SessionSearchComputerState): boolean { return state === 'off' } -export function summarizeSessionSearchComputers( - entries: readonly SessionSearchComputerEntry[] -): SessionSearchFleetSummary { - const count = (state: SessionSearchComputerState): number => - entries.filter((entry) => entry.state === state).length - return { - on: count('on'), - total: entries.length, - offline: count('offline'), - needUpdate: count('needs-update'), - turnOnable: entries.filter((entry) => isTurnOnableSessionSearchState(entry.state)).length - } -} - -/** Sentence above the list. A segment worth zero is left out rather than printed as "0". */ -export function sessionSearchSummarySentence( - summary: SessionSearchFleetSummary, - autoEnableNewComputers: boolean -): string { - const segments = [ - translate('sessionHistory.settings.summaryOn', 'On {{on}} of {{total}} computers', { - on: summary.on, - total: summary.total - }) - ] - if (summary.offline > 0) { - segments.push( - translate('sessionHistory.settings.summaryOffline', '{{offline}} offline', { - offline: summary.offline - }) - ) - } - if (summary.needUpdate > 0) { - segments.push( - translate('sessionHistory.settings.summaryNeedUpdate', '{{needUpdate}} need an update', { - needUpdate: summary.needUpdate - }) - ) - } - const sentence = segments.join(' · ') - return autoEnableNewComputers - ? `${sentence} ${translate('sessionHistory.settings.summaryAutoEnable', 'New computers turn on when they can.')}` - : sentence -} - // Reachable and working first, then what the user could act on, then what they cannot. const STATE_RANK: Record = { on: 0, diff --git a/src/renderer/src/components/settings/use-session-search-auto-enable.ts b/src/renderer/src/components/settings/use-session-search-auto-enable.ts deleted file mode 100644 index 4efdde61db5097e8d973efbc509a2b929ee58c14..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2312 zcmah~O^@3)5bfE&Vv<8FFC4kO6-FH-C=6r|y<``?1Ob+oMl!n+X^@m1!}Wjf8&Z#E`+%;oV5S9GSz*_Nh`RS~8Woh-v~$1q=i=b{VR zn$OmU`*JkS^Tqh*x5{z3;KB6$7Pe!cnDP(XnZV_Bx{r&kwa`o%THM{y7ogBB6Pvwu z*0gK_>2gkC%lMBs9gguug~07DP-AP-o}T&0C2fv5-7OGGmrL@&X;b4xioM5&8>Vn# zRLi5`c`^Zl3Ej~{P^N@071nq`5b1yW*f~SikZPK@OH$<7?b!j10u0dLXv2kV*^@MC zEAZ#yX@{)D@iy zrX5Uo$}sJ<*CD4*$tXf{T%mJbJP;I^lAksbQGqzSn+swEt4OI2(#z=EsuFwy4u#aA z(jt@6mvbwGAua>a*b(43^e3 zNBTjOB%H6`;A73&IqQ~`fqJj%v7_`inXYMOwE94MtI-J9K5z&PLk-#G`+2sE1`(9> zT*feggYqH)em>-phdu~&zal9Ik&|1SrU;*33{5s0EqdLefzbx@mWenJ@wX({583Qu zEwTcd(e(5>9C1u8dC@s1(#2@K?gN*_g6@aevQ^Z=!Mf*pYDkO%wac>f68n%Ib6w8zzpOUd^rz`;0$Le5-tqRR9Xf z2?9dr4VxM?tX3=f`4udcb(AGQDK~*v_+MQ9omQ3QRobhae>6oe<3tOZk7b%B zRPBKULR++;d2qJZ@o!zQcocDg+vnqn==ZS-0DVBJy5 z30U_Y53#12$^RqNDQ@d! zKNPv#MwFzRN|4`c8ehI$EQhDzzi>EE{_Y*ULg&p`jGHYkEJ>e2o@f0BY>Fog@)Iss z+-$g8a1B7&PyIYS&FQe!=qT6Y3udlQ