diff --git a/src/main/persistence-ui-state.test.ts b/src/main/persistence-ui-state.test.ts index d6690712e72..ccafa4e3fab 100644 --- a/src/main/persistence-ui-state.test.ts +++ b/src/main/persistence-ui-state.test.ts @@ -95,6 +95,31 @@ describe('Store', () => { ]) }) + // The RPC now strips manualRepoOrder, so every paired-client ui.set reaches the store without + // the key. Absent has to mean preserve: if it read as clear, the strip would erase the desktop's + // order on the first unrelated setting a phone or web client changes. + // Absent-means-preserve is what makes the pairing-local strip safe: a client's ui.set arrives + // without these fields, so the desktop's own values must survive the update. + it('updateUI preserves the manual repo and host-section order when an update omits them', async () => { + const store = await createStore() + store.updateUI({ + manualRepoOrder: [ + { hostId: 'local', repoId: 'alpha' }, + { hostId: 'ssh:box', repoId: 'bravo' } + ] as never, + workspaceHostOrder: ['ssh:box', 'local'] as never + }) + + store.updateUI({ sidebarWidth: 400 }) + + expect(store.getUI().manualRepoOrder).toEqual([ + { hostId: 'local', repoId: 'alpha' }, + { hostId: 'ssh:box', repoId: 'bravo' } + ]) + expect(store.getUI().workspaceHostOrder).toEqual(['ssh:box', 'local']) + expect(store.getUI().sidebarWidth).toBe(400) + }) + it('updateUI persists sanitized per-worktree dotfile visibility', async () => { const store = await createStore() store.updateUI({ diff --git a/src/main/runtime/rpc/methods/client-ui-pairing-local-fields.test.ts b/src/main/runtime/rpc/methods/client-ui-pairing-local-fields.test.ts new file mode 100644 index 00000000000..d2bcec6a7f3 --- /dev/null +++ b/src/main/runtime/rpc/methods/client-ui-pairing-local-fields.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, vi } from 'vitest' +import { getDefaultUIState } from '../../../../shared/constants' +import { PAIRING_LOCAL_UI_FIELDS } from '../../../../shared/pairing-local-ui-fields' +import type { OrcaRuntimeService } from '../../orca-runtime' +import type { RpcRequest } from '../core' +import { RpcDispatcher } from '../dispatcher' +import { CLIENT_UI_METHODS } from './client-ui' + +function makeRequest(method: string, params?: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +// Both directions of the pairing boundary for the fields in PAIRING_LOCAL_UI_FIELDS: a client's +// value must never be persisted by the host, and the host's must never be returned to a client. +describe('client UI RPC pairing-local field seams', () => { + it('drops a paired client manualRepoOrder while forwarding the rest of the payload', async () => { + const runtime = { + getRuntimeId: () => 'test-runtime', + updateUIState: vi.fn(() => getDefaultUIState()) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + // A paired web client restamps every repo onto its own runtime:web-* pseudo-host, so its + // overlay names hosts this profile will never own; persisting it erases the desktop order. + const response = await dispatcher.dispatch( + makeRequest('ui.set', { + sidebarWidth: 280, + manualRepoOrder: [ + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-a' }, + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-b' } + ] + }) + ) + + expect(response).toMatchObject({ ok: true }) + expect(runtime.updateUIState).toHaveBeenCalledWith({ sidebarWidth: 280 }) + }) + + // Driven off the census so a field added to PAIRING_LOCAL_UI_FIELDS without wiring a seam + // fails here rather than shipping. Sample values are what a paired web client actually sends. + const pairingLocalSamples: Record<(typeof PAIRING_LOCAL_UI_FIELDS)[number], unknown> = { + hideWorkspacesFromOtherDevices: true, + manualRepoOrder: [ + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-a' } + ], + workspaceHostOrder: ['runtime:web-11111111-2222-3333-4444-555555555555', 'local'] + } + + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'ui.set never persists the pairing-local field %s', + async (field) => { + const runtime = { + getRuntimeId: () => 'test-runtime', + updateUIState: vi.fn(() => getDefaultUIState()) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('ui.set', { sidebarWidth: 280, [field]: pairingLocalSamples[field] }) + ) + + expect(response).toMatchObject({ ok: true }) + const forwarded = vi.mocked(runtime.updateUIState).mock.calls[0]?.[0] ?? {} + expect(Object.keys(forwarded)).not.toContain(field) + expect(forwarded).toMatchObject({ sidebarWidth: 280 }) + } + ) + + // The read path matters as much as the write path: a host that echoes these back overwrites the + // client's own value, and on a profile poisoned before the write strip the echoed + // runtime:web-* keys match the very client that minted them. + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'ui.get never returns the pairing-local field %s', + async (field) => { + const runtime = { + getRuntimeId: () => 'test-runtime', + getUIState: vi.fn(() => ({ ...getDefaultUIState(), [field]: pairingLocalSamples[field] })) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + const response = await dispatcher.dispatch(makeRequest('ui.get')) + + const ui = (response as { result: { ui: Record } }).result.ui + expect(Object.keys(ui)).not.toContain(field) + expect(ui).toMatchObject({ sidebarWidth: getDefaultUIState().sidebarWidth }) + } + ) + + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'the ui.set and ui.recordFeatureInteraction responses omit the pairing-local field %s', + async (field) => { + const stored = { ...getDefaultUIState(), [field]: pairingLocalSamples[field] } + const runtime = { + getRuntimeId: () => 'test-runtime', + updateUIState: vi.fn(() => stored), + recordFeatureInteraction: vi.fn(() => stored) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: CLIENT_UI_METHODS }) + + const setResponse = await dispatcher.dispatch(makeRequest('ui.set', { sidebarWidth: 280 })) + const interactionResponse = await dispatcher.dispatch( + makeRequest('ui.recordFeatureInteraction', 'tasks') + ) + + for (const response of [setResponse, interactionResponse]) { + const ui = (response as { result: { ui: Record } }).result.ui + expect(Object.keys(ui)).not.toContain(field) + } + } + ) +}) diff --git a/src/main/runtime/rpc/methods/client-ui.test.ts b/src/main/runtime/rpc/methods/client-ui.test.ts index a1f24c28a9f..3bfb7303bd0 100644 --- a/src/main/runtime/rpc/methods/client-ui.test.ts +++ b/src/main/runtime/rpc/methods/client-ui.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { getDefaultUIState } from '../../../../shared/constants' +import { omitPairingLocalUiFields } from '../../../../shared/pairing-local-ui-fields' import { MAX_QUICK_COMMAND_AGENT_PROMPT_LENGTH, MAX_QUICK_COMMAND_ID_LENGTH, @@ -375,7 +376,7 @@ describe('client UI RPC methods', () => { const response = await dispatcher.dispatch(makeRequest('ui.get')) expect(runtime.getUIState).toHaveBeenCalledTimes(1) - expect(response).toMatchObject({ ok: true, result: { ui } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(ui) } }) }) it('persists UI updates on the runtime host and returns the updated state', async () => { @@ -415,7 +416,7 @@ describe('client UI RPC methods', () => { hideAutomationGeneratedWorkspaces: true, filterRepoIds: ['repo-1'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('lets a paired client clear the OSC 52 default-on notice', async () => { @@ -560,11 +561,12 @@ describe('client UI RPC methods', () => { } const response = await dispatcher.dispatch(makeRequest('ui.set', payload)) + const { manualRepoOrder: _desktopOwnedOrder, ...forwarded } = payload expect(runtime.updateUIState).toHaveBeenCalledWith({ - ...payload, + ...forwarded, worktreeCardProperties: ['status', 'unread', 'branch', 'automation', 'inline-agents'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) // Why one case per field: the schema is strict, so a single unlisted key makes @@ -659,7 +661,7 @@ describe('client UI RPC methods', () => { const response = await dispatcher.dispatch(makeRequest('ui.recordFeatureInteraction', 'tasks')) expect(runtime.recordFeatureInteraction).toHaveBeenCalledWith('tasks') - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('rejects unknown and malformed UI update fields', async () => { @@ -721,7 +723,7 @@ describe('client UI RPC methods', () => { expect(runtime.updateUIState).toHaveBeenCalledWith({ worktreeCardProperties: ['status', 'unread', 'jira-issue'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('accepts every worktree card property the shared union defines', async () => { @@ -801,7 +803,7 @@ describe('client UI RPC methods', () => { expect(runtime.updateUIState).toHaveBeenCalledWith({ worktreeCardProperties: ['status', 'unread', 'ci', 'issue', 'pr'] }) - expect(response).toMatchObject({ ok: true, result: { ui: updated } }) + expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } }) }) it('rejects each star-nag persisted state mutation field from remote clients', async () => { diff --git a/src/main/runtime/rpc/methods/client-ui.ts b/src/main/runtime/rpc/methods/client-ui.ts index 70ad9e9df57..c62328ebc64 100644 --- a/src/main/runtime/rpc/methods/client-ui.ts +++ b/src/main/runtime/rpc/methods/client-ui.ts @@ -1,3 +1,4 @@ +import { omitPairingLocalUiFields } from '../../../../shared/pairing-local-ui-fields' import type { PersistedUIState } from '../../../../shared/persisted-ui-state-types' import { defineMethod, type RpcMethod } from '../core' import { @@ -50,22 +51,24 @@ export const CLIENT_UI_METHODS: RpcMethod[] = [ defineMethod({ name: 'ui.get', params: null, - handler: (_params, { runtime }) => ({ ui: runtime.getUIState() }) + handler: (_params, { runtime }) => ({ ui: omitPairingLocalUiFields(runtime.getUIState()) }) }), defineMethod({ name: 'ui.set', params: UiUpdate, - handler: (params, { runtime }) => { - const { hideWorkspacesFromOtherDevices: _clientLocalFilter, ...hostUpdates } = params - void _clientLocalFilter - return { ui: runtime.updateUIState(hostUpdates as Partial) } - } + // Why the fields are dropped here rather than removed from the schema: UiUpdate is strict, so + // an unlisted key would make the dispatcher reject an old client's ENTIRE payload. + handler: (params, { runtime }) => ({ + ui: omitPairingLocalUiFields( + runtime.updateUIState(omitPairingLocalUiFields(params) as Partial) + ) + }) }), defineMethod({ name: 'ui.recordFeatureInteraction', params: FeatureInteractionIdParam, handler: (params, { runtime }) => ({ - ui: runtime.recordFeatureInteraction(params) + ui: omitPairingLocalUiFields(runtime.recordFeatureInteraction(params)) }) }) ] diff --git a/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts b/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts index 4aa654cf33a..cc13aa983aa 100644 --- a/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts +++ b/src/renderer/src/hooks/direct-ssh-host-hydration.test.ts @@ -113,6 +113,59 @@ describe('createDirectSshHostHydration', () => { ]) }) + it('keeps the manual cross-host order when the host catalog is republished', async () => { + const owner = authority('box') + const store = createStore(() => + state({ + repos: [repo('bravo', 'box'), repo('alpha', null), repo('delta', 'box')], + manualRepoOrder: [ + { hostId: 'ssh:box', repoId: 'bravo' }, + { hostId: 'local', repoId: 'alpha' }, + { hostId: 'ssh:box', repoId: 'delta' } + ] + }) + ) + const hydration = createDirectSshHostHydration({ + store, + listRepos: vi.fn(async () => + hostSnapshot(owner, [repo('bravo', 'box'), repo('delta', 'box')]) + ), + listLineage: vi.fn(), + isCurrentAuthority: () => true + }) + + await hydration.capturePreparationInput(owner, 'reconnect') + + expect(store.getState().repos.map((entry) => entry.id)).toEqual(['bravo', 'alpha', 'delta']) + }) + + // A repo added after the last drag has no overlay entry. Ranked rows must keep their order and + // the newcomer sinks to the tail, rather than the overlay being discarded for being incomplete. + it('keeps ranked rows in order and appends unranked ones when the overlay is partial', async () => { + const owner = authority('box') + const store = createStore(() => + state({ + repos: [repo('bravo', 'box'), repo('alpha', null), repo('delta', 'box')], + manualRepoOrder: [ + { hostId: 'ssh:box', repoId: 'delta' }, + { hostId: 'local', repoId: 'alpha' } + ] + }) + ) + const hydration = createDirectSshHostHydration({ + store, + listRepos: vi.fn(async () => + hostSnapshot(owner, [repo('bravo', 'box'), repo('delta', 'box')]) + ), + listLineage: vi.fn(), + isCurrentAuthority: () => true + }) + + await hydration.capturePreparationInput(owner, 'reconnect') + + expect(store.getState().repos.map((entry) => entry.id)).toEqual(['delta', 'alpha', 'bravo']) + }) + it('rejects a mismatched host response without publishing', async () => { const owner = authority() const store = createStore(() => state({ repos: [repo('cached', 'target-a')] })) diff --git a/src/renderer/src/hooks/direct-ssh-host-hydration.ts b/src/renderer/src/hooks/direct-ssh-host-hydration.ts index 069b3b3d323..419dc134075 100644 --- a/src/renderer/src/hooks/direct-ssh-host-hydration.ts +++ b/src/renderer/src/hooks/direct-ssh-host-hydration.ts @@ -2,6 +2,7 @@ import type { StoreApi } from 'zustand' import { getRepoExecutionHostId, toSshExecutionHostId } from '../../../shared/execution-host' import type { HostLineageSnapshot } from '../../../shared/host-lineage-contract' import type { HostRepoCatalogSnapshot } from '../../../shared/host-repo-catalog-contract' +import { applyManualRepoOrder } from '../../../shared/manual-repo-order' import type { DirectSshAuthority } from '../../../shared/ssh-types' import { isWorkspaceKey } from '../../../shared/workspace-scope' import type { AppState } from '../store/types' @@ -63,12 +64,14 @@ function mergeExactHostCatalog(state: AppState, snapshot: HostRepoCatalogSnapsho return state } const hostId = snapshot.authority.executionHostId + // Why re-apply the overlay: re-appending the host's rows puts them at the tail, which would + // undo the user's manual cross-host order on every connect. return { ...state, - repos: [ - ...state.repos.filter((repo) => getRepoExecutionHostId(repo) !== hostId), - ...snapshot.repos - ] + repos: applyManualRepoOrder( + [...state.repos.filter((repo) => getRepoExecutionHostId(repo) !== hostId), ...snapshot.repos], + state.manualRepoOrder + ) } } diff --git a/src/renderer/src/web/web-preload-api-ui.test.ts b/src/renderer/src/web/web-preload-api-ui.test.ts index 08123524fbd..d958a195063 100644 --- a/src/renderer/src/web/web-preload-api-ui.test.ts +++ b/src/renderer/src/web/web-preload-api-ui.test.ts @@ -1,6 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { FeatureInteractionState } from '../../../shared/feature-interactions' +import { + PAIRING_LOCAL_UI_FIELDS, + type PairingLocalUiField +} from '../../../shared/pairing-local-ui-fields' +import type { PersistedUIState } from '../../../shared/persisted-ui-state-types' import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' +import type { ManualRepoOrderEntry } from '../../../shared/ui-chrome-types' import { installApi, installBrowserGlobals, @@ -317,6 +323,181 @@ describe('web UI preload API', () => { }) }) + it('keeps the manual repo order browser-local instead of overwriting the host order', async () => { + const runtimeCalls: { method: string; params: unknown }[] = [] + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string, params?: unknown): Promise> { + runtimeCalls.push({ method, params }) + return Promise.resolve({ + id: method, + ok: true, + result: { ui: {} }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + // The web client keys its order to its own runtime:web-* host, so a host that predates the + // RPC-side strip would persist it over the desktop's local/ssh-keyed order. + const manualRepoOrder: ManualRepoOrderEntry[] = [ + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-b' }, + { hostId: 'runtime:web-11111111-2222-3333-4444-555555555555', repoId: 'repo-a' } + ] + await globals.window.api.ui.set({ manualRepoOrder, sidebarWidth: 280 }) + + expect(runtimeCalls[0]).toEqual({ method: 'ui.set', params: { sidebarWidth: 280 } }) + expect(JSON.parse(globals.storage.getItem('orca.web.ui.v1') ?? '{}')).toMatchObject({ + manualRepoOrder, + sidebarWidth: 280 + }) + }) + + // Why cover a host that still sends the field: clients and hosts update independently, so a new + // web client meets hosts predating the response omission. On a profile an old web client + // poisoned, those echoed runtime:web-* entries match this browser's own repos and would beat + // every drag it has made since. + it.each([ + [ + 'this browser own stale runtime:web-* entries', + [ + { hostId: 'runtime:web-env-1', repoId: 'repo-c' }, + { hostId: 'runtime:web-env-1', repoId: 'repo-a' } + ] + ], + [ + 'the desktop local/ssh-keyed order', + [ + { hostId: 'local', repoId: 'repo-a' }, + { hostId: 'ssh:box', repoId: 'repo-b' } + ] + ], + ['an empty overlay from a desktop that never dragged', []] + ])('keeps the browser-local manual repo order when ui.get returns %s', async (_label, hosted) => { + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string): Promise> { + return Promise.resolve({ + id: method, + ok: true, + result: { ui: { manualRepoOrder: hosted } }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const webOwnOrder: ManualRepoOrderEntry[] = [ + { hostId: 'runtime:web-env-1', repoId: 'repo-b' }, + { hostId: 'runtime:web-env-1', repoId: 'repo-a' } + ] + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + globals.storage.setItem('orca.web.ui.v1', JSON.stringify({ manualRepoOrder: webOwnOrder })) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + await expect(globals.window.api.ui.get()).resolves.toMatchObject({ + manualRepoOrder: webOwnOrder + }) + expect(JSON.parse(globals.storage.getItem('orca.web.ui.v1') ?? '{}')).toMatchObject({ + manualRepoOrder: webOwnOrder + }) + }) + + it('keeps the browser-local manual repo order when a new host omits it entirely', async () => { + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string): Promise> { + return Promise.resolve({ + id: method, + ok: true, + result: { ui: { sidebarWidth: 320 } }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const webOwnOrder: ManualRepoOrderEntry[] = [ + { hostId: 'runtime:web-env-1', repoId: 'repo-b' }, + { hostId: 'runtime:web-env-1', repoId: 'repo-a' } + ] + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + globals.storage.setItem('orca.web.ui.v1', JSON.stringify({ manualRepoOrder: webOwnOrder })) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + await expect(globals.window.api.ui.get()).resolves.toMatchObject({ + manualRepoOrder: webOwnOrder, + sidebarWidth: 320 + }) + }) + + // Census-driven, matching the host-side seam tests: a field added to PAIRING_LOCAL_UI_FIELDS + // without wiring the web read seam fails here rather than shipping. The host sample differs from + // the browser's for every field, so only the pin makes this pass. + const browserLocalUiSamples: Record = { + hideWorkspacesFromOtherDevices: true, + manualRepoOrder: [{ hostId: 'runtime:web-env-1', repoId: 'repo-b' }], + workspaceHostOrder: ['runtime:web-env-1', 'local'] + } + const hostUiSamples: Record = { + hideWorkspacesFromOtherDevices: false, + manualRepoOrder: [{ hostId: 'local', repoId: 'repo-a' }], + workspaceHostOrder: ['local', 'ssh:box'] + } + + it.each(PAIRING_LOCAL_UI_FIELDS.map((field) => [field] as const))( + 'keeps the browser-local %s and never sends it to the host', + async (field) => { + const runtimeCalls: { method: string; params: unknown }[] = [] + vi.doMock('./web-runtime-client', () => ({ + WebRuntimeClient: class { + call(method: string, params?: unknown): Promise> { + runtimeCalls.push({ method, params }) + return Promise.resolve({ + id: method, + ok: true, + result: { ui: { [field]: hostUiSamples[field] } }, + _meta: { runtimeId: 'runtime-1' } + }) + } + + close(): void {} + } + })) + + const browserLocal = { [field]: browserLocalUiSamples[field] } as Partial + const globals = installBrowserGlobals('Linux') + writeStoredRuntimeEnvironment(globals.storage) + globals.storage.setItem('orca.web.ui.v1', JSON.stringify(browserLocal)) + const { installWebPreloadApi } = await import('./web-preload-api') + installWebPreloadApi() + + await globals.window.api.ui.set({ ...browserLocal, sidebarWidth: 280 }) + + expect(runtimeCalls[0]).toEqual({ method: 'ui.set', params: { sidebarWidth: 280 } }) + await expect(globals.window.api.ui.get()).resolves.toMatchObject(browserLocal) + expect(JSON.parse(globals.storage.getItem('orca.web.ui.v1') ?? '{}')).toMatchObject( + browserLocal + ) + } + ) + it('union-merges local contextual tour seen ids when ui.get returns stale host state', async () => { vi.doMock('./web-runtime-client', () => ({ WebRuntimeClient: class { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 8825898ccdf..0dd0326d1d0 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -32,6 +32,11 @@ import type { } from '../../../shared/global-settings-types' import type { OnboardingState } from '../../../shared/onboarding-state-types' import type { PersistedUIState } from '../../../shared/persisted-ui-state-types' +import { + omitPairingLocalUiFields, + type PairedUiState, + type PairingLocalUiField +} from '../../../shared/pairing-local-ui-fields' import type { MemorySnapshot, StatsSummary } from '../../../shared/process-stats-types' import type { Repo } from '../../../shared/repo-types' import type { @@ -2699,11 +2704,7 @@ function createWebUiApi(): NonNullable['ui']> { return { get: async () => { try { - const result = await callRuntimeResult<{ ui: PersistedUIState }>( - 'ui.get', - undefined, - 15_000 - ) + const result = await callRuntimeResult<{ ui: PairedUiState }>('ui.get', undefined, 15_000) const local = readLocalWebUIState() const next = { ...mergeHostWebUIState(local, result.ui), @@ -2728,9 +2729,9 @@ function createWebUiApi(): NonNullable['ui']> { const next = mergeWebUIState(readLocalWebUIState(), updates) writeJson(UI_STORAGE_KEY, next) zoomLevel = next.uiZoomLevel - const { hideWorkspacesFromOtherDevices: _clientLocalWorkspaceFilter, ...hostUpdates } = - updates - void _clientLocalWorkspaceFilter + // Why strip here too when the host also strips: an old host predating that strip would + // otherwise persist this browser's runtime:web-* keys over the desktop profile's order. + const hostUpdates = omitPairingLocalUiFields(updates) try { await callRuntimeResult('ui.set', hostUpdates, 15_000) } catch { @@ -2752,7 +2753,7 @@ function createWebUiApi(): NonNullable['ui']> { }) writeJson(UI_STORAGE_KEY, optimistic) try { - const result = await callRuntimeResult<{ ui: PersistedUIState }>( + const result = await callRuntimeResult<{ ui: PairedUiState }>( 'ui.recordFeatureInteraction', id, 15_000 @@ -4125,14 +4126,18 @@ function mergeWebUIState( } } -function mergeHostWebUIState( - local: PersistedUIState, - incoming: PersistedUIState -): PersistedUIState { - return { - ...mergeWebUIState(local, incoming), - hideWorkspacesFromOtherDevices: local.hideWorkspacesFromOtherDevices === true - } +// Why pin instead of trusting the host's strip: an old host still returns these fields, and for a +// profile a pre-fix drag poisoned they echo back the same runtime:web-* keys this browser minted, +// which then match its own repos and beat every newer drag. +function mergeHostWebUIState(local: PersistedUIState, incoming: PairedUiState): PersistedUIState { + // Why `satisfies Record<...>` rather than a `Pick<...>` annotation: every member is optional in + // PersistedUIState, so Pick would accept a literal that silently skipped a newly added member. + const pinned = { + hideWorkspacesFromOtherDevices: local.hideWorkspacesFromOtherDevices === true, + manualRepoOrder: local.manualRepoOrder, + workspaceHostOrder: local.workspaceHostOrder + } satisfies Record & Partial + return { ...mergeWebUIState(local, incoming), ...pinned } } function mergeFeatureInteractionState( diff --git a/src/shared/pairing-local-ui-fields.test.ts b/src/shared/pairing-local-ui-fields.test.ts new file mode 100644 index 00000000000..41545abb378 --- /dev/null +++ b/src/shared/pairing-local-ui-fields.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest' +import { omitPairingLocalUiFields, PAIRING_LOCAL_UI_FIELDS } from './pairing-local-ui-fields' + +describe('pairing-local UI fields', () => { + // Why a census: the seams that honor this set are spread across the host RPC and the web + // preload, so a field added here without wiring every seam would otherwise ship silently. + it('census: the set is exactly the fields no pairing may exchange', () => { + expect([...PAIRING_LOCAL_UI_FIELDS]).toEqual([ + 'hideWorkspacesFromOtherDevices', + 'manualRepoOrder', + 'workspaceHostOrder' + ]) + }) + + it('omits every member and keeps everything else', () => { + const state = { + hideWorkspacesFromOtherDevices: true, + manualRepoOrder: [{ hostId: 'local' as const, repoId: 'repo-a' }], + workspaceHostOrder: ['local' as const], + sidebarWidth: 280, + activeView: 'tasks' as const + } + + expect(omitPairingLocalUiFields(state)).toEqual({ sidebarWidth: 280, activeView: 'tasks' }) + }) + + it('leaves a payload that carries no member untouched', () => { + const state = { sidebarWidth: 280 } + + expect(omitPairingLocalUiFields(state)).toEqual(state) + }) + + // A member explicitly set to undefined must still be removed, not forwarded as a present key + // that a receiver would read as "cleared". + it('drops a member present with an undefined value', () => { + expect( + Object.keys(omitPairingLocalUiFields({ manualRepoOrder: undefined, sidebarWidth: 280 })) + ).toEqual(['sidebarWidth']) + }) +}) diff --git a/src/shared/pairing-local-ui-fields.ts b/src/shared/pairing-local-ui-fields.ts new file mode 100644 index 00000000000..453a16eb0ee --- /dev/null +++ b/src/shared/pairing-local-ui-fields.ts @@ -0,0 +1,28 @@ +import type { PersistedUIState } from './persisted-ui-state-types' + +// UI state each side of a pairing owns for itself. These fields describe a client's own view — +// which workspaces it hides, what order it puts repos and host sections in — and are keyed to hosts +// only that side knows about, so a value copied across the boundary in either direction overwrites +// the receiver's answer with one computed for somebody else. They must be stripped on the way to a +// host (ui.set) and pinned to the local value on the way back (ui.get). +export const PAIRING_LOCAL_UI_FIELDS = [ + 'hideWorkspacesFromOtherDevices', + 'manualRepoOrder', + 'workspaceHostOrder' +] as const satisfies readonly (keyof PersistedUIState)[] + +export type PairingLocalUiField = (typeof PAIRING_LOCAL_UI_FIELDS)[number] + +// What a paired client actually receives over the UI RPCs, so reading a pairing-local field off a +// host response is a compile error rather than a silent undefined. +export type PairedUiState = Omit + +const PAIRING_LOCAL_UI_FIELD_SET: ReadonlySet = new Set(PAIRING_LOCAL_UI_FIELDS) + +// Accepts any object, not just Partial: the ui.set seam passes the zod-inferred +// update type, whose optionality differs from the persisted shape. +export function omitPairingLocalUiFields(state: T): Omit { + return Object.fromEntries( + Object.entries(state).filter(([key]) => !PAIRING_LOCAL_UI_FIELD_SET.has(key)) + ) as Omit +}