mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
fix(mobile-sync): stop publishing a group that names a terminal with no rows
A terminal publishes one row per pane, and `getRuntimeLeafIdsForTerminal` returns none when there is no live pane and no persisted layout — which is exactly what `createTerminalTab` seeds (`emptyLayoutSnapshot()`) until TerminalPane mounts. The group still named it, so the snapshot published `tabGroups`, a `tabGroupLayout` leaf and an `activeGroupId` for a tab the client is sent no row for. Off the active worktree, where nothing mounts, that state lasts until the user goes there. Hold such a terminal out of the group projection. The existing machinery does the rest: an emptied group leaves `tabGroups`, its layout leaf is pruned with it, and `resolvePublishedActiveGroupId` moves the pointer. Old clients are unaffected in outcome: `buildHostToLocalTabIdMap` never had a key for the tab, so `buildMirroredHostGroups` already dropped it from the group's order and skipped a group left empty. The host now sends what the client was already computing.
This commit is contained in:
Brennan Benson
committed by
Merge Sim
Merge Sim
parent
d132cc20b5
commit
bfe2a0dabb
@@ -249,6 +249,185 @@ describe('buildMobileSessionTabSnapshots published group references', () => {
|
||||
expect(snapshot?.activeGroupId).toBe('group-right')
|
||||
})
|
||||
|
||||
// Why this arm and not just the mounted one above: a terminal occupies its group under the parent
|
||||
// tab id but publishes one row per pane, so a terminal with no live pane and no persisted layout
|
||||
// publishes none at all. `createTerminalTab` seeds exactly that — `emptyLayoutSnapshot()` — until
|
||||
// TerminalPane mounts, so this is the ordinary new-tab window, not a corrupt fixture.
|
||||
it('keeps a terminal that publishes no surface rows out of its published group', () => {
|
||||
const state = makeState({
|
||||
activeGroupIdByWorktree: { 'wt-1': 'group-1' },
|
||||
groupsByWorktree: {
|
||||
'wt-1': [
|
||||
{
|
||||
id: 'group-1',
|
||||
activeTabId: 'term-unmounted',
|
||||
tabOrder: ['term-unmounted', 'term-mounted'],
|
||||
recentTabIds: ['term-unmounted', 'term-mounted']
|
||||
}
|
||||
]
|
||||
} as unknown as AppState['groupsByWorktree'],
|
||||
layoutByWorktree: {
|
||||
'wt-1': { type: 'leaf', groupId: 'group-1' }
|
||||
} as unknown as AppState['layoutByWorktree'],
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-1': [
|
||||
{
|
||||
id: 'term-unmounted',
|
||||
groupId: 'group-1',
|
||||
contentType: 'terminal',
|
||||
entityId: 'term-unmounted',
|
||||
title: 'Terminal'
|
||||
},
|
||||
{
|
||||
id: 'term-mounted',
|
||||
groupId: 'group-1',
|
||||
contentType: 'terminal',
|
||||
entityId: 'term-mounted',
|
||||
title: 'Terminal'
|
||||
}
|
||||
]
|
||||
} as unknown as AppState['unifiedTabsByWorktree'],
|
||||
tabsByWorktree: {
|
||||
'wt-1': [
|
||||
{
|
||||
id: 'term-unmounted',
|
||||
worktreeId: 'wt-1',
|
||||
ptyId: null,
|
||||
title: 'Terminal',
|
||||
defaultTitle: 'Terminal',
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 1
|
||||
},
|
||||
{
|
||||
id: 'term-mounted',
|
||||
worktreeId: 'wt-1',
|
||||
ptyId: 'pty-mounted',
|
||||
title: 'Terminal',
|
||||
defaultTitle: 'Terminal',
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: 1,
|
||||
createdAt: 2
|
||||
}
|
||||
]
|
||||
} as unknown as AppState['tabsByWorktree'],
|
||||
terminalLayoutsByTabId: {
|
||||
// What `createTerminalTab` writes before the pane mounts: a layout with no leaf at all.
|
||||
'term-unmounted': { root: null, activeLeafId: null, expandedLeafId: null },
|
||||
'term-mounted': {
|
||||
root: { type: 'leaf', leafId: mountedLeafId },
|
||||
activeLeafId: mountedLeafId,
|
||||
expandedLeafId: null,
|
||||
ptyIdsByLeafId: { [mountedLeafId]: 'pty-mounted' }
|
||||
}
|
||||
} as unknown as AppState['terminalLayoutsByTabId']
|
||||
} as unknown as Parameters<typeof makeState>[0])
|
||||
|
||||
const snapshot = buildMobileSessionTabSnapshots(state)[0]
|
||||
|
||||
// The presence half: its neighbour publishes, so a projection that had stopped emitting terminal
|
||||
// rows fails here rather than passing on emptiness.
|
||||
expect(snapshot?.tabs.map((tab) => tab.id)).toEqual([`term-mounted::${mountedLeafId}`])
|
||||
expect(snapshot?.tabGroups).toMatchObject([{ id: 'group-1', tabOrder: ['term-mounted'] }])
|
||||
expect(danglingTabRefs(snapshot)).toEqual([])
|
||||
expect(danglingGroupRefs(snapshot)).toEqual([])
|
||||
})
|
||||
|
||||
// The same terminal alone in its group: holding it back empties the group, so the group and the
|
||||
// pane the layout gave it have to go with it rather than leaving the client an empty split.
|
||||
it('drops a group whose only terminal publishes no surface rows', () => {
|
||||
const state = makeState({
|
||||
activeGroupIdByWorktree: { 'wt-1': 'group-empty' },
|
||||
groupsByWorktree: {
|
||||
'wt-1': [
|
||||
{
|
||||
id: 'group-empty',
|
||||
activeTabId: 'term-unmounted',
|
||||
tabOrder: ['term-unmounted'],
|
||||
recentTabIds: ['term-unmounted']
|
||||
},
|
||||
{
|
||||
id: 'group-right',
|
||||
activeTabId: 'term-mounted',
|
||||
tabOrder: ['term-mounted'],
|
||||
recentTabIds: ['term-mounted']
|
||||
}
|
||||
]
|
||||
} as unknown as AppState['groupsByWorktree'],
|
||||
layoutByWorktree: {
|
||||
'wt-1': {
|
||||
type: 'split',
|
||||
direction: 'horizontal',
|
||||
first: { type: 'leaf', groupId: 'group-empty' },
|
||||
second: { type: 'leaf', groupId: 'group-right' }
|
||||
}
|
||||
} as unknown as AppState['layoutByWorktree'],
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-1': [
|
||||
{
|
||||
id: 'term-unmounted',
|
||||
groupId: 'group-empty',
|
||||
contentType: 'terminal',
|
||||
entityId: 'term-unmounted',
|
||||
title: 'Terminal'
|
||||
},
|
||||
{
|
||||
id: 'term-mounted',
|
||||
groupId: 'group-right',
|
||||
contentType: 'terminal',
|
||||
entityId: 'term-mounted',
|
||||
title: 'Terminal'
|
||||
}
|
||||
]
|
||||
} as unknown as AppState['unifiedTabsByWorktree'],
|
||||
tabsByWorktree: {
|
||||
'wt-1': [
|
||||
{
|
||||
id: 'term-unmounted',
|
||||
worktreeId: 'wt-1',
|
||||
ptyId: null,
|
||||
title: 'Terminal',
|
||||
defaultTitle: 'Terminal',
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 1
|
||||
},
|
||||
{
|
||||
id: 'term-mounted',
|
||||
worktreeId: 'wt-1',
|
||||
ptyId: 'pty-mounted',
|
||||
title: 'Terminal',
|
||||
defaultTitle: 'Terminal',
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: 1,
|
||||
createdAt: 2
|
||||
}
|
||||
]
|
||||
} as unknown as AppState['tabsByWorktree'],
|
||||
terminalLayoutsByTabId: {
|
||||
'term-unmounted': { root: null, activeLeafId: null, expandedLeafId: null },
|
||||
'term-mounted': {
|
||||
root: { type: 'leaf', leafId: mountedLeafId },
|
||||
activeLeafId: mountedLeafId,
|
||||
expandedLeafId: null,
|
||||
ptyIdsByLeafId: { [mountedLeafId]: 'pty-mounted' }
|
||||
}
|
||||
} as unknown as AppState['terminalLayoutsByTabId']
|
||||
} as unknown as Parameters<typeof makeState>[0])
|
||||
|
||||
const snapshot = buildMobileSessionTabSnapshots(state)[0]
|
||||
|
||||
expect(snapshot?.tabGroups?.map((group) => group.id)).toEqual(['group-right'])
|
||||
expect(collectLayoutGroupIds(snapshot?.tabGroupLayout)).toEqual(['group-right'])
|
||||
expect(snapshot?.activeGroupId).toBe('group-right')
|
||||
expect(danglingTabRefs(snapshot)).toEqual([])
|
||||
expect(danglingGroupRefs(snapshot)).toEqual([])
|
||||
})
|
||||
|
||||
// Why this arm: the mirrored-tab guard is applied twice — once choosing which terminals the group
|
||||
// projection may name, once emitting rows. Ablating the projection copy alone leaves the emit copy
|
||||
// holding the tab list, so without this the projection's copy is unpinned and its removal is
|
||||
|
||||
@@ -1305,10 +1305,7 @@ export function buildMobileSessionTabSnapshots(
|
||||
return file ? isMobilePublishableOpenFile(file) : false
|
||||
})
|
||||
const publishableTerminalIds = [...terminalTabByIdForWorktree.values()]
|
||||
.filter(
|
||||
(terminal) =>
|
||||
!isWebOnlyMirroredTerminalTab(terminal, inputs.terminalLayoutByTabId.get(terminal.id))
|
||||
)
|
||||
.filter((terminal) => isMobilePublishableTerminalTab(inputs, terminal))
|
||||
.map((terminal) => terminal.id)
|
||||
const groupProjection = buildMobileSessionGroupProjection(inputs, {
|
||||
terminalIds: publishableTerminalIds,
|
||||
@@ -1917,6 +1914,25 @@ function buildMobileLaunchDraftsByPaneKey(args: {
|
||||
return draftsByPaneKey ?? EMPTY_NARROWED_BY_KEY
|
||||
}
|
||||
|
||||
/**
|
||||
* Why a second guard beside the mirrored-tab one: a terminal occupies its group under the parent tab
|
||||
* id but publishes one row per pane, so one with no live pane and no persisted layout publishes no
|
||||
* rows at all. Leaving it in the group would name a tab the client has no row to resolve it to.
|
||||
*/
|
||||
function isMobilePublishableTerminalTab(
|
||||
inputs: MobileSessionWorktreeInputs,
|
||||
terminal: NonNullable<AppState['tabsByWorktree'][string]>[number]
|
||||
): boolean {
|
||||
const savedLayout = inputs.terminalLayoutByTabId.get(terminal.id)
|
||||
if (isWebOnlyMirroredTerminalTab(terminal, savedLayout)) {
|
||||
return false
|
||||
}
|
||||
return (
|
||||
getRuntimeLeafIdsForTerminal(inputs.mountedSurfaceCaptureByTabId.get(terminal.id), savedLayout)
|
||||
.length > 0
|
||||
)
|
||||
}
|
||||
|
||||
function buildMobileTerminalSurfaceTabs(
|
||||
inputs: MobileSessionWorktreeInputs,
|
||||
terminal: NonNullable<AppState['tabsByWorktree'][string]>[number],
|
||||
|
||||
Reference in New Issue
Block a user