fix(remote): preserve the exact preview source group

This commit is contained in:
Brennan Benson
2026-08-23 13:10:19 -07:00
parent d1127b9939
commit d2575ae386
9 changed files with 292 additions and 72 deletions
@@ -120,6 +120,7 @@ describe('openFileInBrowserTab', () => {
url: 'file:///srv/repo/example.html',
clientTargetGroupId: 'group-2',
clientTargetGroupCreated: true,
clientSourceGroupId: 'group-1',
focusOnCreate: false,
stagedTitle: 'example.html',
stagedFocusAddressBar: false
+1
View File
@@ -225,6 +225,7 @@ export function openFilePreviewToSide(params: {
url: target.url,
clientTargetGroupId: targetGroupId,
clientTargetGroupCreated: !existingSibling,
clientSourceGroupId: sourceGroupId,
focusOnCreate: false,
stagedTitle: target.title,
stagedFocusAddressBar: false
@@ -466,6 +466,7 @@ export async function createWebRuntimeSessionBrowserTab(args: {
targetGroupId?: string
clientTargetGroupId?: string
clientTargetGroupCreated?: boolean
clientSourceGroupId?: string
focusOnCreate?: boolean
/** Wait until a renderer-backed host can publish the new page in its session snapshot. */
waitForRegistration?: boolean
@@ -503,7 +504,9 @@ export async function createWebRuntimeSessionBrowserTab(args: {
worktreeId: args.worktreeId,
remotePageId: provisionalPageId,
groupId: args.clientTargetGroupId,
callerCreatedGroup: args.clientTargetGroupCreated
callerCreatedGroup: args.clientTargetGroupCreated,
sourceGroupId: args.clientSourceGroupId,
focusOwner: intentOwner
})
}
if (shouldSelectWorktree) {
@@ -6,6 +6,8 @@ import {
forgetWebSessionBrowserPlacement,
isWebSessionBrowserPlacementGroupReserved,
peekWebSessionBrowserPlacementGroup,
peekWebSessionBrowserPlacementSourceGroup,
moveWebSessionBrowserPlacement,
recordWebSessionBrowserPlacement,
releaseWebSessionBrowserPlacementGroup,
resetWebSessionBrowserPlacementsForTests,
@@ -18,6 +20,89 @@ const WORKTREE_ID = 'worktree-1'
afterEach(resetWebSessionBrowserPlacementsForTests)
describe('web session browser placement', () => {
it('keeps newest source focus through page remapping and overlapping settlement', () => {
const focusOwner = { environmentId: ENVIRONMENT_ID, pairingRevision: 1 }
recordWebSessionBrowserPlacement({
environmentId: ENVIRONMENT_ID,
worktreeId: WORKTREE_ID,
remotePageId: 'page-1',
groupId: 'preview-group',
sourceGroupId: 'source-group-1',
focusOwner
})
recordWebSessionBrowserPlacement({
environmentId: ENVIRONMENT_ID,
worktreeId: WORKTREE_ID,
remotePageId: 'page-2-provisional',
groupId: 'preview-group',
sourceGroupId: 'source-group-2',
focusOwner
})
moveWebSessionBrowserPlacement({
environmentId: ENVIRONMENT_ID,
worktreeId: WORKTREE_ID,
fromRemotePageId: 'page-2-provisional',
toRemotePageId: 'page-2-host'
})
expect(
peekWebSessionBrowserPlacementSourceGroup({
owner: focusOwner,
worktreeId: WORKTREE_ID,
groupId: 'preview-group'
})
).toBe('source-group-2')
forgetWebSessionBrowserPlacement({
environmentId: ENVIRONMENT_ID,
worktreeId: WORKTREE_ID,
remotePageId: 'page-2-host'
})
expect(
peekWebSessionBrowserPlacementSourceGroup({
owner: focusOwner,
worktreeId: WORKTREE_ID,
groupId: 'preview-group'
})
).toBe('source-group-1')
})
it('does not reuse cleanup-retained focus intent across environments or pairings', () => {
const retiredOwner = { environmentId: ENVIRONMENT_ID, pairingRevision: 1 }
recordWebSessionBrowserPlacement({
environmentId: ENVIRONMENT_ID,
worktreeId: WORKTREE_ID,
remotePageId: 'retired-page',
groupId: 'preview-group',
sourceGroupId: 'retired-source',
focusOwner: retiredOwner,
callerCreatedGroup: true
})
clearWebSessionBrowserPlacementsForEnvironment(ENVIRONMENT_ID)
expect(
peekWebSessionBrowserPlacementSourceGroup({
owner: { environmentId: ENVIRONMENT_ID, pairingRevision: 2 },
worktreeId: WORKTREE_ID,
groupId: 'preview-group'
})
).toBeUndefined()
expect(
peekWebSessionBrowserPlacementSourceGroup({
owner: { environmentId: 'environment-2', pairingRevision: 1 },
worktreeId: WORKTREE_ID,
groupId: 'preview-group'
})
).toBeUndefined()
expect(
peekWebSessionBrowserPlacementSourceGroup({
owner: retiredOwner,
worktreeId: WORKTREE_ID,
groupId: 'preview-group'
})
).toBe('retired-source')
})
it('keeps a shared target group reserved until every pending page settles', () => {
for (const remotePageId of ['page-1', 'page-2']) {
recordWebSessionBrowserPlacement({
@@ -1,12 +1,18 @@
import { webSessionIntentOwnerKey, type WebSessionIntentOwner } from './web-session-intent-owner'
type PendingBrowserPlacement = {
groupId: string
ownsGroupCleanup: boolean
requestSequence: number
sourceGroupId?: string
focusOwnerKey?: string
}
const placementByPendingPage = new Map<string, PendingBrowserPlacement>()
const materializedGroupKeys = new Set<string>()
const pendingCleanupClaimsByGroup = new Map<string, number>()
const MAX_PENDING_PLACEMENTS = 128
let nextPlacementSequence = 0
function pageKey(environmentId: string, worktreeId: string, remotePageId: string): string {
return `${environmentId}\0${worktreeId}\0${remotePageId}`
@@ -43,6 +49,8 @@ export function recordWebSessionBrowserPlacement(args: {
remotePageId: string
groupId: string
callerCreatedGroup?: boolean
sourceGroupId?: string
focusOwner?: WebSessionIntentOwner
}): void {
const key = pageKey(args.environmentId, args.worktreeId, args.remotePageId)
const existing = placementByPendingPage.get(key)
@@ -51,7 +59,18 @@ export function recordWebSessionBrowserPlacement(args: {
}
placementByPendingPage.set(key, {
groupId: args.groupId,
ownsGroupCleanup: args.callerCreatedGroup === true || existing?.ownsGroupCleanup === true
ownsGroupCleanup: args.callerCreatedGroup === true || existing?.ownsGroupCleanup === true,
requestSequence: existing?.requestSequence ?? (nextPlacementSequence += 1),
...(args.sourceGroupId
? { sourceGroupId: args.sourceGroupId }
: existing?.sourceGroupId
? { sourceGroupId: existing.sourceGroupId }
: {}),
...(args.focusOwner
? { focusOwnerKey: webSessionIntentOwnerKey(args.focusOwner) }
: existing?.focusOwnerKey
? { focusOwnerKey: existing.focusOwnerKey }
: {})
})
}
@@ -65,12 +84,13 @@ export function moveWebSessionBrowserPlacement(args: {
const placement = placementByPendingPage.get(fromKey)
placementByPendingPage.delete(fromKey)
if (placement) {
recordWebSessionBrowserPlacement({
environmentId: args.environmentId,
worktreeId: args.worktreeId,
remotePageId: args.toRemotePageId,
groupId: placement.groupId,
callerCreatedGroup: placement.ownsGroupCleanup
const toKey = pageKey(args.environmentId, args.worktreeId, args.toRemotePageId)
const existing = placementByPendingPage.get(toKey)
const newest =
existing && existing.requestSequence > placement.requestSequence ? existing : placement
placementByPendingPage.set(toKey, {
...newest,
ownsGroupCleanup: placement.ownsGroupCleanup || existing?.ownsGroupCleanup === true
})
}
}
@@ -124,6 +144,28 @@ export function isWebSessionBrowserPlacementGroupReserved(args: {
return false
}
export function peekWebSessionBrowserPlacementSourceGroup(args: {
owner: WebSessionIntentOwner
worktreeId: string
groupId: string
}): string | undefined {
const prefix = worktreePrefix(args.owner.environmentId, args.worktreeId)
const focusOwnerKey = webSessionIntentOwnerKey(args.owner)
let latest: PendingBrowserPlacement | undefined
for (const [key, placement] of placementByPendingPage) {
if (
key.startsWith(prefix) &&
placement.groupId === args.groupId &&
placement.focusOwnerKey === focusOwnerKey &&
placement.sourceGroupId &&
(!latest || placement.requestSequence > latest.requestSequence)
) {
latest = placement
}
}
return latest?.sourceGroupId
}
export function releaseWebSessionBrowserPlacementGroup(args: {
environmentId: string
worktreeId: string
@@ -210,4 +252,5 @@ export function resetWebSessionBrowserPlacementsForTests(): void {
placementByPendingPage.clear()
materializedGroupKeys.clear()
pendingCleanupClaimsByGroup.clear()
nextPlacementSequence = 0
}
@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest'
import {
resolveWebSessionSiblingVisibleTabId,
resolveWebSessionGroupVisibleTabId,
resolveWebSessionVisibleTabId
} from './web-session-focus-intent'
import {
@@ -238,8 +238,8 @@ describe('resolveWebSessionVisibleTabId — no-group compatibility path', () =>
})
})
describe('resolveWebSessionSiblingVisibleTabId', () => {
it('returns the sibling whose active tab matches the remembered visible type', () => {
describe('resolveWebSessionGroupVisibleTabId', () => {
it('returns the requested group active tab', () => {
const editor = tab({ id: 'tab-editor', entityId: 'file-1', contentType: 'editor' })
const terminal = tab({
id: 'tab-terminal',
@@ -261,7 +261,20 @@ describe('resolveWebSessionSiblingVisibleTabId', () => {
})
expect(resolveWebSessionVisibleTabId(state, WT)).toBeNull()
expect(resolveWebSessionSiblingVisibleTabId(state, WT)).toBe(editor.id)
expect(resolveWebSessionGroupVisibleTabId(state, WT, GROUP_A)).toBe(editor.id)
})
it('follows a tab rematerialized under a new id within the requested group', () => {
const local = tab({ id: 'tab-local', entityId: 'file-1', contentType: 'editor' })
const mirrored = tab({ id: 'tab-mirrored', entityId: 'file-1', contentType: 'editor' })
const state = makeState({
unifiedTabsByWorktree: { [WT]: [local] },
groupsByWorktree: {
[WT]: [group({ id: GROUP_A, activeTabId: local.id, tabOrder: [local.id] })]
}
})
expect(resolveWebSessionGroupVisibleTabId(state, WT, GROUP_A, [mirrored])).toBe(mirrored.id)
})
})
@@ -46,32 +46,9 @@ export function resolveWebSessionVisibleTabId(
const activeGroupId = state.activeGroupIdByWorktree?.[worktreeId] ?? null
const activeGroup =
(activeGroupId ? groups.find((group) => group.id === activeGroupId) : null) ?? groups[0]
// Why: authoritative that nothing is visible too — never resolve into an unfocused group.
if (activeGroup?.activeTabId == null) {
return null
}
const activeTabId = activeGroup.activeTabId
const direct = tabs.find((tab) => tab.id === activeTabId && tab.groupId === activeGroup.id)
if (direct) {
return direct.id
}
// Why: reconcile can rematerialize the visible tab under a new id (local -> mirrored), so
// follow its entity rather than dropping focus. Stays inside the group to avoid a pane jump.
const previous = (state.unifiedTabsByWorktree?.[worktreeId] ?? []).find(
(tab) => tab.id === activeTabId
)
if (!previous) {
return null
}
const previousType = toVisibleTabType(previous.contentType)
return (
tabs.find(
(tab) =>
tab.groupId === activeGroup.id &&
tab.entityId === previous.entityId &&
toVisibleTabType(tab.contentType) === previousType
)?.id ?? null
)
return activeGroup
? resolveWebSessionGroupVisibleTabId(state, worktreeId, activeGroup.id, tabs)
: null
}
// Why: no group records at all (fresh slice, or first remote reconcile before groups exist).
@@ -95,31 +72,37 @@ export function resolveWebSessionVisibleTabId(
)
}
export function resolveWebSessionSiblingVisibleTabId(
export function resolveWebSessionGroupVisibleTabId(
state: WebSessionVisibleTabState,
worktreeId: string,
groupId: string,
tabs = state.unifiedTabsByWorktree?.[worktreeId] ?? []
): string | null {
const activeGroupId = state.activeGroupIdByWorktree?.[worktreeId] ?? null
const preferredType =
state.activeTabTypeByWorktree?.[worktreeId] ??
(state.activeWorktreeId === worktreeId ? state.activeTabType : null)
const tabById = new Map(tabs.map((tab) => [tab.id, tab]))
let fallback: string | null = null
for (const group of state.groupsByWorktree?.[worktreeId] ?? []) {
if (group.id === activeGroupId || group.activeTabId == null) {
continue
}
const tab = tabById.get(group.activeTabId)
if (!tab || tab.groupId !== group.id) {
continue
}
if (preferredType && toVisibleTabType(tab.contentType) === preferredType) {
return tab.id
}
fallback ??= tab.id
const group = state.groupsByWorktree?.[worktreeId]?.find((candidate) => candidate.id === groupId)
if (group?.activeTabId == null) {
return null
}
return fallback
const direct = tabs.find((tab) => tab.id === group.activeTabId && tab.groupId === group.id)
if (direct) {
return direct.id
}
// Why: reconcile can rematerialize the visible tab under a new id (local -> mirrored), so
// follow its entity rather than dropping focus. Stays inside the group to avoid a pane jump.
const previous = (state.unifiedTabsByWorktree?.[worktreeId] ?? []).find(
(tab) => tab.id === group.activeTabId
)
if (!previous) {
return null
}
const previousType = toVisibleTabType(previous.contentType)
return (
tabs.find(
(tab) =>
tab.groupId === group.id &&
tab.entityId === previous.entityId &&
toVisibleTabType(tab.contentType) === previousType
)?.id ?? null
)
}
function focusIntentPartitionKey(owner: WebSessionIntentOwner, worktreeId: string): string {
@@ -22,8 +22,11 @@ vi.mock('../store', () => ({
const EDITOR_GROUP = 'client-editor-group'
const PREVIEW_GROUP = 'client-preview-group'
const UNRELATED_EDITOR_GROUP = 'unrelated-editor-group'
const EDITOR_FILE_ID = '/repo/paired-html-focus.html'
const EDITOR_TAB_ID = 'local-html-editor'
const UNRELATED_EDITOR_FILE_ID = '/repo/unrelated.html'
const UNRELATED_EDITOR_TAB_ID = 'unrelated-html-editor'
const HOST_TERMINAL_PARENT = 'host-terminal'
const HOST_TERMINAL_SURFACE = `${HOST_TERMINAL_PARENT}::${LEAF_ID}`
const HOST_BROWSER_TAB = 'host-html-browser-tab'
@@ -59,6 +62,16 @@ function htmlEditorTab(): Tab {
}
}
function unrelatedEditorTab(): Tab {
return {
...htmlEditorTab(),
id: UNRELATED_EDITOR_TAB_ID,
entityId: UNRELATED_EDITOR_FILE_ID,
groupId: UNRELATED_EDITOR_GROUP,
label: 'unrelated.html'
}
}
function mirroredTerminalTab(terminalTabId: string): Tab {
return {
id: terminalTabId,
@@ -79,7 +92,14 @@ function mirroredTerminalTab(terminalTabId: string): Tab {
function editorFocusedSplitState(activeGroupId: string): WebSessionTabsSyncState {
const terminalTabId = toWebTerminalSurfaceTabId(HOST_TERMINAL_PARENT)
const editorTab = htmlEditorTab()
const unrelatedTab = unrelatedEditorTab()
const openFile = htmlOpenFile()
const unrelatedOpenFile = {
...openFile,
id: UNRELATED_EDITOR_FILE_ID,
filePath: UNRELATED_EDITOR_FILE_ID,
relativePath: 'unrelated.html'
}
return makeState({
activeFileId: openFile.id,
activeFileIdByWorktree: { [WT]: openFile.id },
@@ -88,7 +108,7 @@ function editorFocusedSplitState(activeGroupId: string): WebSessionTabsSyncState
activeTabIdByWorktree: { [WT]: terminalTabId },
activeTabType: 'editor',
activeTabTypeByWorktree: { [WT]: 'editor' },
openFiles: [openFile],
openFiles: [unrelatedOpenFile, openFile],
tabsByWorktree: {
[WT]: [
{
@@ -103,9 +123,18 @@ function editorFocusedSplitState(activeGroupId: string): WebSessionTabsSyncState
}
]
},
unifiedTabsByWorktree: { [WT]: [mirroredTerminalTab(terminalTabId), editorTab] },
unifiedTabsByWorktree: {
[WT]: [unrelatedTab, mirroredTerminalTab(terminalTabId), editorTab]
},
groupsByWorktree: {
[WT]: [
{
id: UNRELATED_EDITOR_GROUP,
worktreeId: WT,
activeTabId: unrelatedTab.id,
tabOrder: [unrelatedTab.id],
recentTabIds: [unrelatedTab.id]
},
{
id: EDITOR_GROUP,
worktreeId: WT,
@@ -125,8 +154,14 @@ function editorFocusedSplitState(activeGroupId: string): WebSessionTabsSyncState
[WT]: {
type: 'split',
direction: 'horizontal',
first: { type: 'leaf', groupId: EDITOR_GROUP },
second: { type: 'leaf', groupId: PREVIEW_GROUP },
first: { type: 'leaf', groupId: UNRELATED_EDITOR_GROUP },
second: {
type: 'split',
direction: 'horizontal',
first: { type: 'leaf', groupId: EDITOR_GROUP },
second: { type: 'leaf', groupId: PREVIEW_GROUP },
ratio: 0.5
},
ratio: 0.5
}
}
@@ -201,6 +236,8 @@ describe('applyWebSessionTabsSnapshot — unfocused HTML side preview', () => {
worktreeId: WT,
remotePageId: HOST_BROWSER_PAGE,
groupId: PREVIEW_GROUP,
sourceGroupId: EDITOR_GROUP,
focusOwner: { environmentId: ENV },
callerCreatedGroup: true
})
const state = editorFocusedSplitState(PREVIEW_GROUP)
@@ -238,12 +275,54 @@ describe('applyWebSessionTabsSnapshot — unfocused HTML side preview', () => {
expect(previewFocus(next).activeTabType).toBe('editor')
})
it('does not fall back to an unrelated editor after the source group closes', () => {
recordWebSessionBrowserPlacement({
environmentId: ENV,
worktreeId: WT,
remotePageId: HOST_BROWSER_PAGE,
groupId: PREVIEW_GROUP,
sourceGroupId: EDITOR_GROUP,
focusOwner: { environmentId: ENV },
callerCreatedGroup: true
})
const state = editorFocusedSplitState(PREVIEW_GROUP)
state.groupsByWorktree = {
[WT]: (state.groupsByWorktree[WT] ?? []).filter((group) => group.id !== EDITOR_GROUP)
}
state.unifiedTabsByWorktree = {
[WT]: (state.unifiedTabsByWorktree[WT] ?? []).filter((tab) => tab.groupId !== EDITOR_GROUP)
}
state.openFiles = state.openFiles.filter((file) => file.id !== EDITOR_FILE_ID)
state.layoutByWorktree = {
[WT]: {
type: 'split',
direction: 'horizontal',
first: { type: 'leaf', groupId: UNRELATED_EDITOR_GROUP },
second: { type: 'leaf', groupId: PREVIEW_GROUP },
ratio: 0.5
}
}
const patch = applyWebSessionTabsSnapshot(
state,
unfocusedHtmlPreviewSnapshot(),
ENV,
NOW + 10
) as Partial<WebSessionTabsSyncState>
const next = { ...state, ...patch }
expect(previewFocus(next).activeGroupId).not.toBe(UNRELATED_EDITOR_GROUP)
expect(previewFocus(next).activeTabId).not.toBe(UNRELATED_EDITOR_TAB_ID)
})
it('keeps the source editor focused when the editor group is still active', () => {
recordWebSessionBrowserPlacement({
environmentId: ENV,
worktreeId: WT,
remotePageId: HOST_BROWSER_PAGE,
groupId: PREVIEW_GROUP,
sourceGroupId: EDITOR_GROUP,
focusOwner: { environmentId: ENV },
callerCreatedGroup: true
})
const state = editorFocusedSplitState(EDITOR_GROUP)
@@ -64,7 +64,7 @@ import {
clearWebSessionFocusIntent,
clearWebSessionFocusIntentsForOwner,
peekWebSessionFocusIntent,
resolveWebSessionSiblingVisibleTabId,
resolveWebSessionGroupVisibleTabId,
resolveWebSessionVisibleTabId
} from './web-session-focus-intent'
import {
@@ -105,6 +105,7 @@ import {
clearWebSessionBrowserPlacementsForEnvironment,
clearWebSessionBrowserPlacementsForWorktree,
isWebSessionBrowserPlacementGroupReserved,
peekWebSessionBrowserPlacementSourceGroup,
peekWebSessionBrowserPlacementGroup,
resetWebSessionBrowserPlacementsForTests
} from './web-session-browser-placement'
@@ -2910,15 +2911,26 @@ function applyWebSessionTabsSnapshotWithContext(
nextUnifiedTabs ?? []
)
const activeGroupId = state.activeGroupIdByWorktree[worktreeId]
// Why: Open Preview to the Side can activate an empty reserved group before the host
// browser lands. A snapshot that still has the host terminal active must not treat
// that emptiness as a terminal focus change.
const reservedEmptyPreviewFallbackTabId =
currentVisibleUnifiedTabId == null &&
activeGroupId != null &&
isWebSessionBrowserPlacementGroupReserved({ worktreeId, groupId: activeGroupId })
? resolveWebSessionSiblingVisibleTabId(state, worktreeId, nextUnifiedTabs ?? [])
: null
// Why: pointer/AT focus can enter the pending empty preview, so restore its recorded source instead of inferring from the host terminal or another editor group.
const reservedPreviewSourceGroupId =
currentVisibleUnifiedTabId == null && activeGroupId != null
? peekWebSessionBrowserPlacementSourceGroup({
owner: {
environmentId,
pairingRevision: getRuntimeEnvironmentRevision(environmentId)
},
worktreeId,
groupId: activeGroupId
})
: undefined
const reservedEmptyPreviewFallbackTabId = reservedPreviewSourceGroupId
? resolveWebSessionGroupVisibleTabId(
state,
worktreeId,
reservedPreviewSourceGroupId,
nextUnifiedTabs ?? []
)
: null
// Why: a client-initiated activation also drives the visible unified tab, overriding the sticky current-visible tab.
const intentUnifiedTabId = honorSnapshotActiveFocus
? navigationIntentTab?.type === 'browser'