diff --git a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-groups.ts b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-groups.ts index d47d119646c..22d6eb2d79b 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-groups.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-groups.ts @@ -141,7 +141,8 @@ export function prepareWebSessionTabsSnapshotGroups( validUnifiedTabIds, environmentId, worktreeId, - clientGroupIdByLocalTabId + clientGroupIdByLocalTabId, + honorSnapshotActiveFocus }) } const strippedGroups = retainClientPlacedMirroredTabs({ diff --git a/src/renderer/src/runtime/web-session-tabs-sync/layout-groups.ts b/src/renderer/src/runtime/web-session-tabs-sync/layout-groups.ts index d85b7c6954a..0d59578335a 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/layout-groups.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/layout-groups.ts @@ -128,7 +128,8 @@ export function buildMirroredHostGroups({ validUnifiedTabIds, environmentId, worktreeId, - clientGroupIdByLocalTabId + clientGroupIdByLocalTabId, + honorSnapshotActiveFocus }: { currentGroups: readonly TabGroup[] hostGroups: readonly RuntimeMobileSessionTabGroup[] @@ -140,6 +141,9 @@ export function buildMirroredHostGroups({ environmentId: string worktreeId: string clientGroupIdByLocalTabId: ReadonlyMap + /** True only when this frame carries navigation intent — a client focus request, or a host + * `navigationIntent: 'follow'`. Unsolicited frames never move a client's focus (#5435). */ + honorSnapshotActiveFocus: boolean }): TabGroup[] | null { const strippedGroups = retainClientPlacedMirroredTabs({ groups: currentGroups, @@ -149,6 +153,12 @@ export function buildMirroredHostGroups({ nextActiveUnifiedTabId }) const groupsById = new Map(strippedGroups.map((group) => [group.id, group])) + // Why pre-strip: stripping drops every mirrored tab this client did not place itself, so a group + // whose tabs are all mirrored comes back with `activeTabId: null` and its focus looks unheld. + // What the client was showing is only legible before that. + const clientActiveTabIdByGroupId = new Map( + currentGroups.map((group) => [group.id, group.activeTabId]) + ) const orderedGroups: TabGroup[] = [] const seen = new Set() @@ -180,14 +190,26 @@ export function buildMirroredHostGroups({ } const activeFromHost = hostGroup.activeTabId !== null ? (hostToLocalTabId.get(hostGroup.activeTabId) ?? null) : null + // Why this order: `activeFromHost` reports which tab the HOST has focused, which on a host + // running an agent follows the working session. It answers "where is the host looking", not + // "where should this client look", so it outranks the tab this client is showing only when the + // frame carries intent. Without that, every republication repointed each group the client was + // not currently visiting. + const heldTabId = existing?.activeTabId ?? clientActiveTabIdByGroupId.get(hostGroup.id) ?? null + const clientActiveTabId = heldTabId && tabOrder.includes(heldTabId) ? heldTabId : null + const hostActiveTabId = + activeFromHost && tabOrder.includes(activeFromHost) ? activeFromHost : null const activeTabId = - nextActiveUnifiedTabId && tabOrder.includes(nextActiveUnifiedTabId) + (nextActiveUnifiedTabId && tabOrder.includes(nextActiveUnifiedTabId) ? nextActiveUnifiedTabId - : activeFromHost && tabOrder.includes(activeFromHost) - ? activeFromHost - : existing?.activeTabId && tabOrder.includes(existing.activeTabId) - ? existing.activeTabId - : (tabOrder[0] ?? null) + : null) ?? + (honorSnapshotActiveFocus ? hostActiveTabId : null) ?? + // A group this client has never shown has no focus to preserve, so the host's is the only + // answer available — that is adoption, not an override. + clientActiveTabId ?? + hostActiveTabId ?? + tabOrder[0] ?? + null orderedGroups.push({ id: hostGroup.id, worktreeId, diff --git a/src/renderer/src/runtime/web-session-tabs-sync/mirrored-group-active-tab-ranking.test.ts b/src/renderer/src/runtime/web-session-tabs-sync/mirrored-group-active-tab-ranking.test.ts new file mode 100644 index 00000000000..bd69f7b5f6c --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-sync/mirrored-group-active-tab-ranking.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest' +import type { TabGroup } from '../../../../shared/tab-types' +import { buildMirroredHostGroups } from './layout-groups' + +// Why: #5435's rule, applied one level down. A host group's `activeTabId` is a report of what the +// host has focused — on an agent-running host it follows the working session — so it must not +// outrank the tab this client is showing in that group. Only a client-owned intent may. + +const WT = 'repo::/worktree' +const ENV = 'web-env-1' +const NOW = 1_700_000_000_000 + +const GROUP_A = 'host-group-a' +const GROUP_B = 'host-group-b' +const VISIBLE_TAB = 'web-terminal-visible' +const CLIENT_TAB_IN_B = 'web-terminal-client-choice' +const HOST_ACTIVE_TAB_IN_B = 'web-terminal-host-choice' + +function group(id: string, activeTabId: string | null, tabOrder: string[]): TabGroup { + return { id, worktreeId: WT, activeTabId, tabOrder, recentTabIds: [...tabOrder] } +} + +/** Two groups, as a split workspace has: the client is looking at group A, and group B shows a tab + * of its own while the host reports a different one as active. */ +function buildSplit(options: { honorSnapshotActiveFocus: boolean }): TabGroup[] | null { + return buildMirroredHostGroups({ + currentGroups: [ + group(GROUP_A, VISIBLE_TAB, [VISIBLE_TAB]), + group(GROUP_B, CLIENT_TAB_IN_B, [CLIENT_TAB_IN_B, HOST_ACTIVE_TAB_IN_B]) + ], + hostGroups: [ + { id: GROUP_A, activeTabId: 'host-visible', tabOrder: ['host-visible'] }, + { + id: GROUP_B, + activeTabId: 'host-choice', + tabOrder: ['host-client-choice', 'host-choice'] + } + ], + hostToLocalTabId: new Map([ + ['host-visible', VISIBLE_TAB], + ['host-client-choice', CLIENT_TAB_IN_B], + ['host-choice', HOST_ACTIVE_TAB_IN_B] + ]), + mirroredUnifiedIds: new Set([VISIBLE_TAB, CLIENT_TAB_IN_B, HOST_ACTIVE_TAB_IN_B]), + // The client is looking at group A, so nothing selects a tab inside group B. + nextActiveUnifiedTabId: VISIBLE_TAB, + now: NOW, + validUnifiedTabIds: new Set([VISIBLE_TAB, CLIENT_TAB_IN_B, HOST_ACTIVE_TAB_IN_B]), + environmentId: ENV, + worktreeId: WT, + clientGroupIdByLocalTabId: new Map(), + honorSnapshotActiveFocus: options.honorSnapshotActiveFocus + }) +} + +describe('buildMirroredHostGroups — active tab ranking', () => { + it('keeps the tab the client is showing in a group the host reports differently', () => { + const groups = buildSplit({ honorSnapshotActiveFocus: false }) + + expect(groups?.find((entry) => entry.id === GROUP_A)?.activeTabId).toBe(VISIBLE_TAB) + expect(groups?.find((entry) => entry.id === GROUP_B)?.activeTabId).toBe(CLIENT_TAB_IN_B) + }) + + it('follows the host active tab when the frame carries navigation intent', () => { + const groups = buildSplit({ honorSnapshotActiveFocus: true }) + + expect(groups?.find((entry) => entry.id === GROUP_B)?.activeTabId).toBe(HOST_ACTIVE_TAB_IN_B) + }) + + it('adopts the host active tab for a group this client has never shown', () => { + const groups = buildMirroredHostGroups({ + currentGroups: [group(GROUP_A, VISIBLE_TAB, [VISIBLE_TAB])], + hostGroups: [ + { id: GROUP_A, activeTabId: 'host-visible', tabOrder: ['host-visible'] }, + { id: GROUP_B, activeTabId: 'host-choice', tabOrder: ['host-choice'] } + ], + hostToLocalTabId: new Map([ + ['host-visible', VISIBLE_TAB], + ['host-choice', HOST_ACTIVE_TAB_IN_B] + ]), + mirroredUnifiedIds: new Set([VISIBLE_TAB, HOST_ACTIVE_TAB_IN_B]), + nextActiveUnifiedTabId: VISIBLE_TAB, + now: NOW, + validUnifiedTabIds: new Set([VISIBLE_TAB, HOST_ACTIVE_TAB_IN_B]), + environmentId: ENV, + worktreeId: WT, + clientGroupIdByLocalTabId: new Map(), + honorSnapshotActiveFocus: false + }) + + expect(groups?.find((entry) => entry.id === GROUP_B)?.activeTabId).toBe(HOST_ACTIVE_TAB_IN_B) + }) +})