From 42a4320ae1062607ce4596a0e83ca32168aa25ad Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 16 Sep 2026 02:04:12 -0400 Subject: [PATCH] fix(settings): say how many computers Turn on would reach Drops the summary sentence: every row already states whether it is offline or needs an update, so counting those again above the list said nothing new. What is left is the one thing the list cannot say, the size of the action, carried by the button's own label. With nothing left to turn on, the standing consent speaks in its place, and only when it is armed. --- .../SessionHistorySettingsPane.test.tsx | 50 ++++++++++------ .../settings/SessionHistorySettingsPane.tsx | 26 +++++---- .../session-search-computer-rollup.test.ts | 43 ++++---------- .../session-search-computer-rollup.ts | 57 ++----------------- src/renderer/src/i18n/locales/en.json | 9 +-- 5 files changed, 67 insertions(+), 118 deletions(-) diff --git a/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx b/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx index 270b651e24d..5975d946f7d 100644 --- a/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx +++ b/src/renderer/src/components/settings/SessionHistorySettingsPane.test.tsx @@ -110,7 +110,7 @@ function statusByHost(): void { return answer }) } -const summaryLine = (): string => screen.getByText(/computers/).textContent ?? '' +const turnOnButton = (name: string | RegExp) => screen.queryByRole('button', { name }) async function openAdvanced(): Promise { await act(async () => { fireEvent.click(screen.getByRole('button', { name: /Advanced/ })) @@ -359,7 +359,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(turnOnButton(/Turn on for all/)).not.toBeInTheDocument() expect(screen.queryByRole('button', { name: 'Open' })).not.toBeInTheDocument() expect(mocks.status).not.toHaveBeenCalled() }) @@ -391,25 +391,30 @@ 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(turnOnButton(/Turn on for all/)).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('names in the button how many computers it would actually turn on', 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 } + // Of five computers only gpu-a is reachable, new enough and off; the offline and + // too-old ones are not counted, and this computer and build-01 are already on. + expect(turnOnButton('Turn on for all 1')).toBeInTheDocument() + + mocks.environments = [ + { id: 'a', name: 'gpu-a' }, + { id: 'b', name: 'gpu-b' } + ] + mocks.details = { a: CONNECTED_DETAILS, b: CONNECTED_DETAILS } + mocks.statusByHost = { local: off, 'runtime:a': off, 'runtime:b': off } statusByHost() cleanup() - pane(true) + pane(false) await act(async () => {}) - expect(summaryLine()).toBe('On 1 of 2 computers') + expect(turnOnButton('Turn on for all 3')).toBeInTheDocument() }) it('turns on every reachable computer and skips the ones it cannot', async () => { @@ -419,7 +424,7 @@ it('turns on every reachable computer and skips the ones it cannot', async () => pane(true, confirm, save) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) + fireEvent.click(screen.getByRole('button', { name: 'Turn on for all 1' })) }) expect(confirm).not.toHaveBeenCalled() expect(mocks.setEnabled.mock.calls.map((call) => call[0])).toEqual(['runtime:off']) @@ -435,7 +440,7 @@ it('turns this computer on as part of turning them all on', async () => { pane(false, vi.fn().mockResolvedValue(true), save) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) + fireEvent.click(screen.getByRole('button', { name: 'Turn on for all 2' })) }) expect(save).toHaveBeenCalledWith({ aiVaultSearch: { enabled: true, historyDays: null } }) expect(mocks.setEnabled).toHaveBeenCalledWith('runtime:off', true) @@ -454,14 +459,14 @@ it('keeps going after a host refuses, and withholds the standing consent', async pane(true, vi.fn().mockResolvedValue(true), save) await act(async () => {}) await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Turn on all' })) + fireEvent.click(screen.getByRole('button', { name: 'Turn on for all 2' })) }) 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 () => { +it('replaces the button with the standing promise once nothing is left to turn on', async () => { mocks.environments = [ { id: 'a', name: 'gpu-a' }, { id: 'gone', name: 'linux 1' } @@ -471,8 +476,19 @@ it('hides the button and says so once nothing is left to turn on', async () => { 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.') + expect(turnOnButton(/Turn on for all/)).not.toBeInTheDocument() + expect(screen.getByText('New computers turn on automatically.')).toBeInTheDocument() +}) + +it('stays silent about automatic turn-on when that consent was never given', async () => { + mocks.environments = [{ id: 'a', name: 'gpu-a' }] + mocks.details = { a: CONNECTED_DETAILS } + mocks.statusByHost = { local: current, 'runtime:a': current } + statusByHost() + pane(true) + await act(async () => {}) + expect(turnOnButton(/Turn on for all/)).not.toBeInTheDocument() + expect(screen.queryByText('New computers turn on automatically.')).not.toBeInTheDocument() }) it('turns on a newly reachable server while the standing consent holds', async () => { diff --git a/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx b/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx index b2ec893f0a0..16256298985 100644 --- a/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx +++ b/src/renderer/src/components/settings/SessionHistorySettingsPane.tsx @@ -20,10 +20,9 @@ import { SessionHistoryComputerRow } from './SessionHistoryComputerRow' import { SessionHistoryServerRow } from './SessionHistoryServerRow' import { SessionSearchComputerList } from './SessionSearchComputerList' import { + countTurnOnableSessionSearchComputers, isTurnOnableSessionSearchState, orderSessionSearchServers, - sessionSearchSummarySentence, - summarizeSessionSearchComputers, type SessionSearchComputerEntry, type SessionSearchComputerState } from './session-search-computer-rollup' @@ -82,7 +81,7 @@ export function SessionHistorySettingsPane({ state: serverStates[environment.id] ?? 'checking', environment })) - const summary = summarizeSessionSearchComputers([localEntry, ...serverEntries]) + const turnOnableCount = countTurnOnableSessionSearchComputers([localEntry, ...serverEntries]) const orderedServers = orderSessionSearchServers(serverEntries) // Rebuilt each render on purpose: the hook keys off the host ids, not this array. const autoEnableTargets = serverEntries @@ -230,13 +229,11 @@ export function SessionHistorySettingsPane({ )}

- {/* With no paired server the line and the button only restate the single switch below them. */} + {/* With no paired server this row only restates the single switch below it. Each row + already says whether it is offline or too old, so nothing here counts those again. */} {serverEntries.length === 0 ? null : ( -
-

- {sessionSearchSummarySentence(summary, autoEnableNewComputers)} -

- {summary.turnOnable > 0 ? ( +
+ {turnOnableCount > 0 ? ( + ) : autoEnableNewComputers ? ( +

+ {translate( + 'sessionHistory.settings.autoEnableArmed', + 'New computers turn on automatically.' + )} +

) : 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..f2cd784ec45 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,11 @@ -import { expect, it, vi } from 'vitest' +import { expect, it } from 'vitest' import { + countTurnOnableSessionSearchComputers, 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,14 +16,15 @@ 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('counts only the computers a turn-on would actually reach', () => { + expect(countTurnOnableSessionSearchComputers(fleet)).toBe(1) + expect(countTurnOnableSessionSearchComputers([])).toBe(0) + expect( + countTurnOnableSessionSearchComputers([ + { id: 'a', name: 'a', state: 'off' }, + { id: 'b', name: 'b', state: 'off' } + ]) + ).toBe(2) }) it('will not offer to turn on a computer it cannot reach or that is too old', () => { @@ -39,22 +34,6 @@ it('will not offer to turn on a computer it cannot reach or that is too old', () } }) -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') -}) - it('orders reachable and working first, then by name inside each group', () => { const ordered = orderSessionSearchServers([ { id: 'f', name: 'probing', state: 'checking' }, 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..a7a9414d7df 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,62 +12,15 @@ 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( +/** How many computers Turn on would actually reach: reachable, new enough, and still off. */ +export function countTurnOnableSessionSearchComputers( 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 +): number { + return entries.filter((entry) => isTurnOnableSessionSearchState(entry.state)).length } // Reachable and working first, then what the user could act on, then what they cannot. diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index b115ca1a2d5..fd51caec022 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -17979,16 +17979,13 @@ "clearedAndTurnedOff": "Search turned off and search data cleared.", "thisComputer": "This computer", "remoteServers": "Orca remote servers", - "summaryOn": "On {{on}} of {{total}} computers", - "summaryOffline": "{{offline}} offline", - "summaryNeedUpdate": "{{needUpdate}} need an update", - "summaryAutoEnable": "New computers turn on when they can.", - "turnOnAll": "Turn on all", + "turnOnAll": "Turn on for all {{count}}", "showMore": "Show {{count}} more", "showFewer": "Show fewer", "openInSidebar": "Open in the sidebar", "openInSidebarCopy": "Type what you remember, or ask an agent: “find the session where we fixed the login timeout.”", - "open": "Open" + "open": "Open", + "autoEnableArmed": "New computers turn on automatically." } }, "aiVault": {