fix(repos): stop the host echoing pairing-local UI state back to clients (STA-4850)

Stripping manualRepoOrder on the write path left the read path open. ui.get
still returned the host's array, and the web client's merge let it win. On a
profile an old web client had already poisoned, the runtime:web-<uuid> entries
echoed back to the very browser that minted them — the environment id is
persisted per pairing — so they matched its restamped repos and overrode every
drag it made after. Even on a clean profile the desktop-keyed array (or [])
replaced the browser's own order on each reload.

Close both ends: the host omits these fields from every ui read response, and
the web client pins its local values when merging one. Either half alone fixes
one skew direction; both are needed because clients and hosts update apart.

Replace the hand-maintained destructures with PAIRING_LOCAL_UI_FIELDS, one
exported set consumed at all four seams, so a field added to it cannot be wired
into some seams and forgotten in others. A census test pins the membership and
the seam tests iterate it. hideWorkspacesFromOtherDevices joins the set with no
behavior change: the web client already pinned it locally, and mobile's only
ui.get consumer reads trustedOrcaHooks.

Also pin the preserve branch in the store: an update that omits manualRepoOrder
must keep the stored order, since after the strip that is every paired ui.set.
This commit is contained in:
Jinwoo-H
2026-08-19 19:09:52 -04:00
parent 0735811b38
commit c46fbcf9c4
9 changed files with 336 additions and 56 deletions
+21
View File
@@ -95,6 +95,27 @@ 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.
it('updateUI preserves the manual repo order when an update omits it', async () => {
const store = await createStore()
store.updateUI({
manualRepoOrder: [
{ hostId: 'local', repoId: 'alpha' },
{ hostId: 'ssh:box', repoId: 'bravo' }
] as never
})
store.updateUI({ sidebarWidth: 400 })
expect(store.getUI().manualRepoOrder).toEqual([
{ hostId: 'local', repoId: 'alpha' },
{ hostId: 'ssh:box', repoId: 'bravo' }
])
expect(store.getUI().sidebarWidth).toBe(400)
})
it('updateUI persists sanitized per-worktree dotfile visibility', async () => {
const store = await createStore()
store.updateUI({
@@ -0,0 +1,110 @@
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' }
]
}
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<string, unknown> } }).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<string, unknown> } }).result.ui
expect(Object.keys(ui)).not.toContain(field)
}
}
)
})
+7 -29
View File
@@ -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 () => {
@@ -565,30 +566,7 @@ describe('client UI RPC methods', () => {
...forwarded,
worktreeCardProperties: ['status', 'unread', 'branch', 'automation', 'inline-agents']
})
expect(response).toMatchObject({ ok: true, result: { ui: updated } })
})
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 })
expect(response).toMatchObject({ ok: true, result: { ui: omitPairingLocalUiFields(updated) } })
})
// Why one case per field: the schema is strict, so a single unlisted key makes
@@ -683,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 () => {
@@ -745,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 () => {
@@ -825,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 () => {
+10 -15
View File
@@ -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,30 +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,
// Why manualRepoOrder is dropped rather than removed from the schema: a paired client keys
// its overlay to its own execution hosts, so forwarding it blind-replaces the desktop's
// order. The strict schema still has to accept the field or old clients' whole payload fails.
handler: (params, { runtime }) => {
const {
hideWorkspacesFromOtherDevices: _clientLocalFilter,
manualRepoOrder: _desktopOwnedOrder,
...hostUpdates
} = params
void _clientLocalFilter
void _desktopOwnedOrder
return { ui: runtime.updateUIState(hostUpdates as Partial<PersistedUIState>) }
}
// 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<PersistedUIState>)
)
})
}),
defineMethod({
name: 'ui.recordFeatureInteraction',
params: FeatureInteractionIdParam,
handler: (params, { runtime }) => ({
ui: runtime.recordFeatureInteraction(params)
ui: omitPairingLocalUiFields(runtime.recordFeatureInteraction(params))
})
})
]
@@ -139,6 +139,33 @@ describe('createDirectSshHostHydration', () => {
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<AppState>(() =>
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<AppState>(() => state({ repos: [repo('cached', 'target-a')] }))
@@ -366,6 +366,92 @@ describe('web UI preload API', () => {
})
})
// 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<RuntimeRpcResponse<unknown>> {
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<RuntimeRpcResponse<unknown>> {
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
})
})
it('union-merges local contextual tour seen ids when ui.get returns stale host state', async () => {
vi.doMock('./web-runtime-client', () => ({
WebRuntimeClient: class {
+14 -12
View File
@@ -32,6 +32,10 @@ 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 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 {
@@ -2731,15 +2735,9 @@ function createWebUiApi(): NonNullable<Partial<PreloadApi>['ui']> {
const next = mergeWebUIState(readLocalWebUIState(), updates)
writeJson(UI_STORAGE_KEY, next)
zoomLevel = next.uiZoomLevel
// Why manualRepoOrder stays local: it is keyed to this client's runtime:web-* hosts, so
// sending it would overwrite the desktop profile's order on hosts that predate the strip.
const {
hideWorkspacesFromOtherDevices: _clientLocalWorkspaceFilter,
manualRepoOrder: _desktopOwnedOrder,
...hostUpdates
} = updates
void _clientLocalWorkspaceFilter
void _desktopOwnedOrder
// 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 {
@@ -4172,14 +4170,18 @@ function mergeWebUIState(
}
}
// 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: PersistedUIState
): PersistedUIState {
return {
...mergeWebUIState(local, incoming),
hideWorkspacesFromOtherDevices: local.hideWorkspacesFromOtherDevices === true
const pinned: Pick<PersistedUIState, PairingLocalUiField> = {
hideWorkspacesFromOtherDevices: local.hideWorkspacesFromOtherDevices === true,
manualRepoOrder: local.manualRepoOrder
}
return { ...mergeWebUIState(local, incoming), ...pinned }
}
function mergeFeatureInteractionState(
@@ -0,0 +1,38 @@
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'
])
})
it('omits every member and keeps everything else', () => {
const state = {
hideWorkspacesFromOtherDevices: true,
manualRepoOrder: [{ hostId: 'local' as const, repoId: 'repo-a' }],
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'])
})
})
+23
View File
@@ -0,0 +1,23 @@
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 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'
] as const satisfies readonly (keyof PersistedUIState)[]
export type PairingLocalUiField = (typeof PAIRING_LOCAL_UI_FIELDS)[number]
const PAIRING_LOCAL_UI_FIELD_SET: ReadonlySet<string> = new Set(PAIRING_LOCAL_UI_FIELDS)
// Accepts any object, not just Partial<PersistedUIState>: the ui.set seam passes the zod-inferred
// update type, whose optionality differs from the persisted shape.
export function omitPairingLocalUiFields<T extends object>(state: T): Omit<T, PairingLocalUiField> {
return Object.fromEntries(
Object.entries(state).filter(([key]) => !PAIRING_LOCAL_UI_FIELD_SET.has(key))
) as Omit<T, PairingLocalUiField>
}