fix(orcad-migration): a sidebar view setting never blocks moving an SSH host to its server (#31374)

* fix(orcad-migration): a sidebar view preference never blocks moving an SSH host to its server

* fix(orcad-migration): keep repo filters on the desktop, keep later acknowledgements; e2e repro
This commit is contained in:
Neil
2026-10-20 21:15:56 -07:00
committed by GitHub
parent ce71535bbf
commit b3e6705337
6 changed files with 195 additions and 147 deletions
@@ -1,5 +1,6 @@
import { describe, expect, it } from 'vitest'
import type { PersistedState } from '../../../shared/persisted-state-types'
import { getDefaultUIState } from '../../../shared/constants'
import type { OrcadMigrationClientStatePayload } from '../../../shared/orcad-migration/orcad-migration-client-state'
import {
applyPreparedOrcadMigrationClientState,
@@ -66,7 +67,7 @@ describe('destination client-state migration', () => {
expect(destination.ui).toMatchObject({
lastActiveRepoId: 'repo-1',
lastActiveWorktreeId: 'repo-1::/worktree',
filterRepoIds: ['repo-1'],
filterRepoIds: [],
showDotfilesByWorktree: { 'repo-1::/worktree': false }
})
})
@@ -101,11 +102,93 @@ describe('destination client-state migration', () => {
)
})
it('rejects non-default UI routing conflicts', () => {
it("moves onto a fresh server whose sidebar shows all hosts, keeping the server's filters", () => {
const destination = state()
destination.ui.lastActiveRepoId = 'other-repo'
expect(() => prepareOrcadMigrationClientState(incoming, destination)).toThrow(
'orcad_migration_client_state_conflict:ui:lastActiveRepoId'
// The server's defaults: 'all' and an all-hosts automation filter, not unset.
destination.ui = getDefaultUIState()
const prepared = prepareOrcadMigrationClientState(
{
uiRouting: {
workspaceHostScope: 'local',
visibleWorkspaceHostIds: ['local'],
workspaceHostOrder: ['local'],
automationHostFilter: { kind: 'host', hostKey: 'host:runtime:environment-1:self' }
}
},
destination
)
applyPreparedOrcadMigrationClientState(prepared, destination)
expect(destination.ui).toMatchObject({
workspaceHostScope: 'all',
visibleWorkspaceHostIds: null,
workspaceHostOrder: [],
automationHostFilter: { kind: 'all' }
})
})
it("keeps the server's own view preferences instead of refusing the move", () => {
const destination = state()
destination.ui = {
...destination.ui,
lastActiveRepoId: 'server-repo',
lastActiveWorktreeId: 'server-repo::/w',
filterRepoIds: ['server-repo'],
showDotfilesByWorktree: { 'server-repo::/w': true, 'repo-1::/worktree': true },
setupScriptPromptDismissedRepoIds: ['server-repo'],
manualRepoOrder: [{ hostId: 'local', repoId: 'server-repo' }],
workspaceHostScope: 'all',
acknowledgedAgentsByPaneKey: { 'server-tab:leaf': 5, 'tab-1:leaf': 9 }
}
const prepared = prepareOrcadMigrationClientState(
{
uiRouting: {
...incoming.uiRouting,
setupScriptPromptDismissedRepoIds: ['repo-1'],
acknowledgedAgentsByPaneKey: { 'tab-1:leaf': 1, 'tab-2:leaf': 2 }
}
},
destination
)
applyPreparedOrcadMigrationClientState(prepared, destination)
expect(destination.ui).toMatchObject({
lastActiveRepoId: 'server-repo',
lastActiveWorktreeId: 'server-repo::/w',
filterRepoIds: ['server-repo'],
showDotfilesByWorktree: { 'server-repo::/w': true, 'repo-1::/worktree': true },
setupScriptPromptDismissedRepoIds: ['server-repo', 'repo-1'],
manualRepoOrder: [{ hostId: 'local', repoId: 'server-repo' }],
acknowledgedAgentsByPaneKey: { 'server-tab:leaf': 5, 'tab-1:leaf': 9, 'tab-2:leaf': 2 }
})
})
it("ignores an older desktop's host filters on a server already scoped elsewhere", () => {
const destination = state()
destination.ui = {
...getDefaultUIState(),
workspaceHostScope: 'runtime:other',
visibleWorkspaceHostIds: ['runtime:other'],
workspaceHostOrder: ['runtime:other'],
automationHostFilter: { kind: 'host', hostKey: 'host:desktop:self' },
filterRepoIds: ['server-repo'],
acknowledgedAgentsByPaneKey: { 'tab-1:leaf': 1 }
}
const before = structuredClone(destination.ui)
applyPreparedOrcadMigrationClientState(
prepareOrcadMigrationClientState(
{
uiRouting: {
workspaceHostScope: 'local',
visibleWorkspaceHostIds: ['local'],
workspaceHostOrder: ['local'],
automationHostFilter: { kind: 'host', hostKey: 'host:runtime:environment-1:self' },
filterRepoIds: ['repo-1'],
acknowledgedAgentsByPaneKey: { 'tab-1:leaf': 7 }
}
},
destination
),
destination
)
expect(destination.ui).toEqual({ ...before, acknowledgedAgentsByPaneKey: { 'tab-1:leaf': 7 } })
})
})
@@ -1,5 +1,4 @@
import type { PersistedState } from '../../../shared/persisted-state-types'
import { hostStableKey, parseHostStableKey } from '../../../shared/automation-owner-key'
import type {
OrcadMigrationClientStatePayload,
OrcadMigrationUiRoutingState
@@ -22,10 +21,7 @@ export function prepareOrcadMigrationClientState(
incoming?.mobileClientTabSelectionsByDeviceId,
state
)
const uiRouting = incoming?.uiRouting ? normalizeUiRouting(incoming.uiRouting) : undefined
if (uiRouting) {
assertUiRoutingCompatible(uiRouting, state)
}
const uiRouting = incoming?.uiRouting ? structuredClone(incoming.uiRouting) : undefined
return {
incoming,
mobileSelections,
@@ -52,24 +48,6 @@ export function applyPreparedOrcadMigrationClientState(
}
}
function normalizeUiRouting(route: OrcadMigrationUiRoutingState): OrcadMigrationUiRoutingState {
const filter = route.automationHostFilter
if (filter?.kind !== 'host') {
return structuredClone(route)
}
const parsed = parseHostStableKey(filter.hostKey)
if (parsed?.authority.kind !== 'runtime' || parsed.selector.kind !== 'self') {
return structuredClone(route)
}
return {
...structuredClone(route),
automationHostFilter: {
kind: 'host',
hostKey: hostStableKey({ authority: { kind: 'desktop' }, selector: { kind: 'self' } })
}
}
}
function prepareMobileSelections(
incoming: OrcadMigrationClientStatePayload['mobileClientTabSelectionsByDeviceId'],
state: PersistedState
@@ -94,88 +72,76 @@ function prepareMobileSelections(
return result
}
// What each routed field reads in this profile, and what it holds when never set.
const UI_ROUTING_FIELDS: readonly {
key: keyof OrcadMigrationUiRoutingState
read: (ui: PersistedState['ui']) => unknown
empty: unknown
}[] = [
{ key: 'lastActiveRepoId', read: (ui) => ui.lastActiveRepoId, empty: null },
{ key: 'lastActiveWorktreeId', read: (ui) => ui.lastActiveWorktreeId, empty: null },
{ key: 'filterRepoIds', read: (ui) => ui.filterRepoIds, empty: [] },
{ key: 'showDotfilesByWorktree', read: (ui) => ui.showDotfilesByWorktree ?? {}, empty: {} },
{
key: 'setupScriptPromptDismissedRepoIds',
read: (ui) => ui.setupScriptPromptDismissedRepoIds ?? [],
empty: []
},
{ key: 'manualRepoOrder', read: (ui) => ui.manualRepoOrder ?? [], empty: [] },
{ key: 'workspaceHostScope', read: (ui) => ui.workspaceHostScope, empty: undefined },
{ key: 'visibleWorkspaceHostIds', read: (ui) => ui.visibleWorkspaceHostIds, empty: null },
{ key: 'workspaceHostOrder', read: (ui) => ui.workspaceHostOrder ?? [], empty: [] },
{ key: 'automationHostFilter', read: (ui) => ui.automationHostFilter, empty: undefined },
{
key: 'acknowledgedAgentsByPaneKey',
read: (ui) => ui.acknowledgedAgentsByPaneKey ?? {},
empty: {}
}
]
type UiRoutingKey = keyof OrcadMigrationUiRoutingState
type UiRoutingFieldMerge<K extends UiRoutingKey> =
/** A filter of one desktop window (its hosts, its repos); the server keeps its own. Ignored here. */
| 'desktop-only'
/** Folds a moved value into this profile; the server's own value always stands. */
| ((ui: PersistedState['ui'], value: NonNullable<OrcadMigrationUiRoutingState[K]>) => void)
function assertUiRoutingCompatible(
route: OrcadMigrationUiRoutingState,
state: PersistedState
): void {
for (const { key, read, empty } of UI_ROUTING_FIELDS) {
const current = serializeOrcadMigrationValue(read(state.ui))
if (
route[key] !== undefined &&
current !== serializeOrcadMigrationValue(route[key]) &&
current !== serializeOrcadMigrationValue(empty)
) {
throw new Error(`orcad_migration_client_state_conflict:ui:${key}`)
/**
* Every moved sidebar field is per-window view state, so none may refuse a move: what the server
* already shows stands, and the desktop's value fills only what the server never set.
* Acknowledgements keep the later time. A new field
* must pick its merge here; refusals belong to catalog and session identity, never to view state.
*/
export const ORCAD_MIGRATION_UI_ROUTING_MERGE: {
readonly [K in UiRoutingKey]: UiRoutingFieldMerge<K>
} = {
lastActiveRepoId: (ui, value) => {
ui.lastActiveRepoId ??= value
},
lastActiveWorktreeId: (ui, value) => {
ui.lastActiveWorktreeId ??= value
},
manualRepoOrder: (ui, value) => {
if (!ui.manualRepoOrder?.length) {
ui.manualRepoOrder = structuredClone(value)
}
}
},
showDotfilesByWorktree: (ui, value) => {
ui.showDotfilesByWorktree = { ...structuredClone(value), ...ui.showDotfilesByWorktree }
},
acknowledgedAgentsByPaneKey: (ui, value) => {
// The later acknowledgement covers the earlier one, so a retry never revives an unread mark.
const merged = { ...ui.acknowledgedAgentsByPaneKey }
for (const [paneKey, at] of Object.entries(value)) {
merged[paneKey] = Math.max(merged[paneKey] ?? 0, at)
}
ui.acknowledgedAgentsByPaneKey = merged
},
setupScriptPromptDismissedRepoIds: (ui, value) => {
ui.setupScriptPromptDismissedRepoIds = [
...new Set([...(ui.setupScriptPromptDismissedRepoIds ?? []), ...value])
]
},
filterRepoIds: 'desktop-only',
workspaceHostScope: 'desktop-only',
visibleWorkspaceHostIds: 'desktop-only',
workspaceHostOrder: 'desktop-only',
automationHostFilter: 'desktop-only'
}
const UI_ROUTING_KEYS = Object.keys(ORCAD_MIGRATION_UI_ROUTING_MERGE).filter(isUiRoutingKey)
function isUiRoutingKey(key: string): key is UiRoutingKey {
return Object.hasOwn(ORCAD_MIGRATION_UI_ROUTING_MERGE, key)
}
function applyUiRouting(route: OrcadMigrationUiRoutingState, state: PersistedState): void {
const ui = state.ui
if (route.lastActiveRepoId !== undefined) {
ui.lastActiveRepoId = route.lastActiveRepoId
}
if (route.lastActiveWorktreeId !== undefined) {
ui.lastActiveWorktreeId = route.lastActiveWorktreeId
}
if (route.filterRepoIds !== undefined) {
ui.filterRepoIds = structuredClone(route.filterRepoIds)
}
if (route.showDotfilesByWorktree !== undefined) {
ui.showDotfilesByWorktree = {
...ui.showDotfilesByWorktree,
...structuredClone(route.showDotfilesByWorktree)
}
}
if (route.setupScriptPromptDismissedRepoIds !== undefined) {
ui.setupScriptPromptDismissedRepoIds = structuredClone(route.setupScriptPromptDismissedRepoIds)
}
if (route.manualRepoOrder !== undefined) {
ui.manualRepoOrder = structuredClone(route.manualRepoOrder)
}
if (route.workspaceHostScope !== undefined) {
ui.workspaceHostScope = route.workspaceHostScope
}
if (route.visibleWorkspaceHostIds !== undefined) {
ui.visibleWorkspaceHostIds = structuredClone(route.visibleWorkspaceHostIds)
}
if (route.workspaceHostOrder !== undefined) {
ui.workspaceHostOrder = structuredClone(route.workspaceHostOrder)
}
if (route.automationHostFilter !== undefined) {
ui.automationHostFilter = structuredClone(route.automationHostFilter)
}
if (route.acknowledgedAgentsByPaneKey !== undefined) {
ui.acknowledgedAgentsByPaneKey = {
...ui.acknowledgedAgentsByPaneKey,
...structuredClone(route.acknowledgedAgentsByPaneKey)
}
for (const key of UI_ROUTING_KEYS) {
mergeUiRoutingField(key, route, state.ui)
}
}
function mergeUiRoutingField<K extends UiRoutingKey>(
key: K,
route: OrcadMigrationUiRoutingState,
ui: PersistedState['ui']
): void {
const merge = ORCAD_MIGRATION_UI_ROUTING_MERGE[key]
const value = route[key]
if (merge !== 'desktop-only' && value !== undefined && value !== null) {
merge(ui, value)
}
}
@@ -10,6 +10,7 @@ import {
} from '../../../shared/client-hosted-browser-page-record'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
import { collectOrcadMigrationSourceClientState } from './orcad-source-client-state'
import { ORCAD_MIGRATION_UI_ROUTING_MERGE } from './orcad-destination-client-state'
const source = {
sshTargetId: 'target-1',
@@ -135,11 +136,34 @@ describe('source client-state migration', () => {
expect(result.payload?.uiRouting).toMatchObject({
lastActiveRepoId: 'repo-1',
lastActiveWorktreeId: 'repo-1::/srv/worktree',
workspaceHostScope: 'local',
showDotfilesByWorktree: { 'repo-1::/srv/worktree': false }
})
})
it("never sends the desktop's sidebar filters, which an older server refuses when they differ", () => {
const current = state()
current.ui.workspaceHostScope = 'ssh:target-1'
current.ui.visibleWorkspaceHostIds = ['ssh:target-1']
current.ui.workspaceHostOrder = ['ssh:target-1']
current.ui.automationHostFilter = { kind: 'host', hostKey: 'host:desktop:ssh:target-1' }
current.ui.filterRepoIds = ['repo-1']
current.ui.lastActiveRepoId = 'repo-1'
const result = collectOrcadMigrationSourceClientState(
current,
source,
catalog,
'environment-1',
undefined
)
expect(result.payload?.uiRouting).toEqual({ lastActiveRepoId: 'repo-1' })
for (const [key, merge] of Object.entries(ORCAD_MIGRATION_UI_ROUTING_MERGE)) {
if (merge === 'desktop-only') {
expect(result.payload?.uiRouting).not.toHaveProperty(key)
}
}
})
it('leaves behind a mobile selection that points at a group outside the migrated owner', () => {
const current = state()
const sourceOwner = 'ssh:target-1|repo-1::/srv/worktree'
@@ -1,5 +1,3 @@
import { hostStableKey } from '../../../shared/automation-owner-key'
import { parsePersistedAutomationHostFilter } from '../../../shared/automation-host-filter'
import type {
OrcadMigrationClientStatePayload,
OrcadMigrationUiRoutingState
@@ -42,7 +40,7 @@ export function collectOrcadMigrationSourceClientState(
const blockedCounts = { 'saved-port-forward': 0 }
let blockedCount = 0
const mobile = collectMobileSelections(state, scope, eligibleSession)
const uiRouting = collectUiRouting(state, scope, destinationEnvironmentId, eligibleSession)
const uiRouting = collectUiRouting(state, scope, eligibleSession)
const target = state.sshTargets.find((entry) => entry.id === scope.targetId)
const savedPortForwards = target?.portForwards ? structuredClone(target.portForwards) : undefined
if (savedPortForwards) {
@@ -119,7 +117,6 @@ function collectMobileSelections(
function collectUiRouting(
state: PersistedState,
scope: ReturnType<typeof createOrcadMigrationSourceScope>,
destinationEnvironmentId: string | undefined,
session: WorkspaceSessionState | undefined
): OrcadMigrationUiRoutingState | undefined {
const ui = state.ui
@@ -130,10 +127,6 @@ function collectUiRouting(
if (ui.lastActiveWorktreeId && orcadMigrationOwnerMatchesScope(ui.lastActiveWorktreeId, scope)) {
result.lastActiveWorktreeId = unqualifyOrcadMigrationOwnerKey(ui.lastActiveWorktreeId)
}
const filterRepoIds = ui.filterRepoIds.filter((repoId) => orcadMigrationOwnsRepoId(scope, repoId))
if (filterRepoIds.length > 0) {
result.filterRepoIds = filterRepoIds
}
const dotfiles = Object.entries(ui.showDotfilesByWorktree ?? {})
.filter(([ownerKey]) => orcadMigrationOwnerMatchesScope(ownerKey, scope))
.map(([ownerKey, enabled]) => [unqualifyOrcadMigrationOwnerKey(ownerKey), enabled] as const)
@@ -152,34 +145,7 @@ function collectUiRouting(
if (manualOrder.length > 0) {
result.manualRepoOrder = manualOrder
}
if (ui.workspaceHostScope === scope.hostId) {
result.workspaceHostScope = 'local'
}
const visibleHosts = (ui.visibleWorkspaceHostIds ?? []).filter(
(hostId) => hostId === scope.hostId
)
if (visibleHosts.length > 0) {
result.visibleWorkspaceHostIds = ['local']
}
const hostOrder = (ui.workspaceHostOrder ?? []).filter((hostId) => hostId === scope.hostId)
if (hostOrder.length > 0) {
result.workspaceHostOrder = ['local']
}
const filter = parsePersistedAutomationHostFilter(ui.automationHostFilter)
if (
filter.kind === 'host' &&
hostStableKey(filter.host) === `host:desktop:ssh:${encodeURIComponent(scope.targetId)}`
) {
result.automationHostFilter = destinationEnvironmentId
? {
kind: 'host',
hostKey: hostStableKey({
authority: { kind: 'runtime', environmentId: destinationEnvironmentId },
selector: { kind: 'self' }
})
}
: { kind: 'host', hostKey: 'host:desktop:self' }
}
// Sidebar filters stay behind: they belong to this desktop's window (ORCAD_MIGRATION_UI_ROUTING_MERGE).
const eligible = session ? collectEligibleSessionIdentity(session, scope) : null
const sourceTabIds = collectSourceOwnedTabIds(state, scope)
const acknowledgements = Object.entries(ui.acknowledgedAgentsByPaneKey ?? {}).filter(
@@ -94,6 +94,8 @@ export function seedRelayEraProfile(
relayPtyId?: string
/** An unsaved buffer on the editor tab, as hot exit persists it. */
draftContent?: string
/** The sidebar shows only this host, which a server's own all-hosts default once refused. */
sidebarScopedToHost?: boolean
} = {}
): RelayEraProfile {
const now = Date.now()
@@ -280,6 +282,10 @@ export function seedRelayEraProfile(
lastDetachedAt: now
})
}
if (options.sidebarScopedToHost) {
const ui = state.ui
state.ui = { ...(ui && typeof ui === 'object' ? ui : {}), workspaceHostScope: hostId }
}
const partitions = state.workspaceSessionsByHostId
state.workspaceSessionsByHostId = {
...(partitions && typeof partitions === 'object' ? partitions : {}),
@@ -119,10 +119,13 @@ test('a relay-era desktop joins the server another version runs for the same log
await waitForSessionReady(established.page)
await desktopB.close(app)
app = null
const seeded = seedRelayEraProfile(desktopB.userDataDir, host.input, {
repoPath: host.remoteRepoPath,
folderPath: host.remoteFolderPath
})
// B's sidebar shows only this host; A's server keeps its own all-hosts view.
const seeded = seedRelayEraProfile(
desktopB.userDataDir,
host.input,
{ repoPath: host.remoteRepoPath, folderPath: host.remoteFolderPath },
{ sidebarScopedToHost: true }
)
const b = await desktopB.launch()
app = b.app
await waitForSessionReady(b.page)