mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 08:03:33 +00:00
fix(terminal): stop canonical PTY filter from evicting live legacy rows
Two regressions in the hydrateWorkspaceSession canonical-PTY filter: 1. canonicalPtyIds was built before isValidTerminalTabId ran, so a web-client mirror row with a "::" id seeded its PTY into the set and then evicted the valid local row owning the same PTY - wiping every terminal in the worktree and orphaning the daemon PTY, persisted back to disk on the next write. 2. `.some()` dropped an entire legacy split tab for a single overlapping leaf, discarding panes whose PTYs nothing else referenced. Only rows that survive the id check now claim ownership, and only pure duplicates (every PTY already canonical, guarded against PTY-less rows) drop.
This commit is contained in:
@@ -0,0 +1,176 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { Tab, TerminalTab, WorkspaceSessionState } from '../../../../shared/types'
|
||||
import { getDefaultWorkspaceSession } from '../../../../shared/constants'
|
||||
import { buildWorkspaceSessionPayload } from '@/lib/workspace-session'
|
||||
import { createTestStore, makeLayout, makeTab, makeWorktree, seedStore } from './store-test-helpers'
|
||||
|
||||
vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } }))
|
||||
vi.mock('@/runtime/sync-runtime-graph', () => ({
|
||||
scheduleRuntimeGraphSync: vi.fn()
|
||||
}))
|
||||
vi.mock('@/components/terminal-pane/pty-transport', () => ({
|
||||
registerEagerPtyBuffer: vi.fn(),
|
||||
ensurePtyDispatcher: vi.fn()
|
||||
}))
|
||||
|
||||
const apiProxy = (): unknown =>
|
||||
new Proxy(() => undefined, {
|
||||
get: (_target, prop) => (prop === 'then' ? undefined : apiProxy()),
|
||||
apply: () => Promise.resolve(null)
|
||||
})
|
||||
|
||||
// @ts-expect-error -- mocked browser preload API
|
||||
globalThis.window = { api: apiProxy() }
|
||||
|
||||
const WORKTREE_ID = 'repo1::/wt-1'
|
||||
|
||||
function makeCanonicalUnifiedTab(entityId: string, sortOrder: number): Tab {
|
||||
return {
|
||||
id: `unified-${entityId}`,
|
||||
entityId,
|
||||
groupId: 'group-1',
|
||||
worktreeId: WORKTREE_ID,
|
||||
contentType: 'terminal',
|
||||
label: 'Grok',
|
||||
customLabel: null,
|
||||
color: null,
|
||||
sortOrder,
|
||||
createdAt: 1
|
||||
}
|
||||
}
|
||||
|
||||
function makeSession(args: {
|
||||
tabs: TerminalTab[]
|
||||
layouts: WorkspaceSessionState['terminalLayoutsByTabId']
|
||||
remoteSessionIdsByTabId?: Record<string, string>
|
||||
canonicalEntityIds: string[]
|
||||
}): WorkspaceSessionState {
|
||||
const unifiedTabs = args.canonicalEntityIds.map((entityId, index) =>
|
||||
makeCanonicalUnifiedTab(entityId, index)
|
||||
)
|
||||
return {
|
||||
...getDefaultWorkspaceSession(),
|
||||
activeRepoId: 'repo1',
|
||||
activeWorktreeId: WORKTREE_ID,
|
||||
activeWorktreeIdsOnShutdown: [WORKTREE_ID],
|
||||
tabsByWorktree: { [WORKTREE_ID]: args.tabs },
|
||||
terminalLayoutsByTabId: args.layouts,
|
||||
remoteSessionIdsByTabId: args.remoteSessionIdsByTabId,
|
||||
unifiedTabs: { [WORKTREE_ID]: unifiedTabs },
|
||||
tabGroups: {
|
||||
[WORKTREE_ID]: [
|
||||
{
|
||||
id: 'group-1',
|
||||
worktreeId: WORKTREE_ID,
|
||||
activeTabId: unifiedTabs[0]?.id ?? null,
|
||||
tabOrder: unifiedTabs.map((tab) => tab.id)
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function hydrate(session: WorkspaceSessionState): ReturnType<typeof createTestStore> {
|
||||
const store = createTestStore()
|
||||
seedStore(store, {
|
||||
worktreesByRepo: {
|
||||
repo1: [makeWorktree({ id: WORKTREE_ID, repoId: 'repo1', path: '/wt-1' })]
|
||||
}
|
||||
})
|
||||
store.getState().hydrateWorkspaceSession(session)
|
||||
store.getState().hydrateTabsSession(session)
|
||||
return store
|
||||
}
|
||||
|
||||
describe('hydrateWorkspaceSession canonical PTY overlap', () => {
|
||||
it('keeps the valid local row when an invalid-id canonical mirror shares its PTY', () => {
|
||||
const sharedPtyId = 'daemon-session-1'
|
||||
const mirrorTabId = 'host-tab::11111111-1111-4111-8111-111111111111'
|
||||
const session = makeSession({
|
||||
tabs: [
|
||||
makeTab({ id: mirrorTabId, worktreeId: WORKTREE_ID, ptyId: sharedPtyId }),
|
||||
makeTab({ id: 'local-tab', worktreeId: WORKTREE_ID, ptyId: sharedPtyId, sortOrder: 1 })
|
||||
],
|
||||
layouts: {
|
||||
[mirrorTabId]: { ...makeLayout(), ptyIdsByLeafId: { 'mirror-leaf': sharedPtyId } },
|
||||
'local-tab': { ...makeLayout(), ptyIdsByLeafId: { 'local-leaf': sharedPtyId } }
|
||||
},
|
||||
canonicalEntityIds: [mirrorTabId]
|
||||
})
|
||||
|
||||
const state = hydrate(session).getState()
|
||||
const persisted = buildWorkspaceSessionPayload(state)
|
||||
|
||||
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual(['local-tab'])
|
||||
expect(state.pendingReconnectPtyIdByTabId['local-tab']).toBe(sharedPtyId)
|
||||
expect(persisted.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual(['local-tab'])
|
||||
expect(persisted.terminalLayoutsByTabId[mirrorTabId]).toBeUndefined()
|
||||
})
|
||||
|
||||
it('keeps a legacy split tab whose second pane owns an independent PTY', () => {
|
||||
const sharedPtyId = 'daemon-shared'
|
||||
const soloPtyId = 'daemon-solo'
|
||||
const session = makeSession({
|
||||
tabs: [
|
||||
makeTab({ id: 'canonical-tab', worktreeId: WORKTREE_ID, ptyId: sharedPtyId }),
|
||||
makeTab({ id: 'split-tab', worktreeId: WORKTREE_ID, ptyId: soloPtyId, sortOrder: 1 })
|
||||
],
|
||||
layouts: {
|
||||
'canonical-tab': { ...makeLayout(), ptyIdsByLeafId: { 'canonical-leaf': sharedPtyId } },
|
||||
'split-tab': {
|
||||
root: {
|
||||
type: 'split',
|
||||
direction: 'vertical',
|
||||
first: { type: 'leaf', leafId: 'leaf-a' },
|
||||
second: { type: 'leaf', leafId: 'leaf-b' }
|
||||
},
|
||||
activeLeafId: 'leaf-a',
|
||||
expandedLeafId: null,
|
||||
ptyIdsByLeafId: { 'leaf-a': sharedPtyId, 'leaf-b': soloPtyId }
|
||||
}
|
||||
},
|
||||
canonicalEntityIds: ['canonical-tab']
|
||||
})
|
||||
|
||||
const state = hydrate(session).getState()
|
||||
const persisted = buildWorkspaceSessionPayload(state)
|
||||
|
||||
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
|
||||
'canonical-tab',
|
||||
'split-tab'
|
||||
])
|
||||
expect(state.pendingReconnectPtyIdByTabId['split-tab']).toBe(soloPtyId)
|
||||
expect(
|
||||
Object.values(state.terminalLayoutsByTabId['split-tab']?.ptyIdsByLeafId ?? {})
|
||||
).toContain(soloPtyId)
|
||||
expect(persisted.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
|
||||
'canonical-tab',
|
||||
'split-tab'
|
||||
])
|
||||
expect(
|
||||
Object.values(persisted.terminalLayoutsByTabId['split-tab']?.ptyIdsByLeafId ?? {})
|
||||
).toContain(soloPtyId)
|
||||
})
|
||||
|
||||
it('retains a non-canonical row that owns no PTY at all', () => {
|
||||
const sharedPtyId = 'daemon-shared'
|
||||
const session = makeSession({
|
||||
tabs: [
|
||||
makeTab({ id: 'canonical-tab', worktreeId: WORKTREE_ID, ptyId: sharedPtyId }),
|
||||
makeTab({ id: 'empty-tab', worktreeId: WORKTREE_ID, ptyId: null, sortOrder: 1 })
|
||||
],
|
||||
layouts: {
|
||||
'canonical-tab': { ...makeLayout(), ptyIdsByLeafId: { 'canonical-leaf': sharedPtyId } },
|
||||
'empty-tab': makeLayout()
|
||||
},
|
||||
canonicalEntityIds: ['canonical-tab']
|
||||
})
|
||||
|
||||
const state = hydrate(session).getState()
|
||||
|
||||
expect(state.tabsByWorktree[WORKTREE_ID]?.map((tab) => tab.id)).toEqual([
|
||||
'canonical-tab',
|
||||
'empty-tab'
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -3959,9 +3959,10 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
||||
tab.contentType === 'terminal' ? [tab.entityId] : []
|
||||
)
|
||||
)
|
||||
// Why: only rows that survive the id check can claim PTY ownership; a dropped invalid-id mirror must not evict the valid row sharing its PTY.
|
||||
const canonicalPtyIds = new Set(
|
||||
tabs
|
||||
.filter((tab) => canonicalTerminalIds.has(tab.id))
|
||||
.filter((tab) => canonicalTerminalIds.has(tab.id) && isValidTerminalTabId(tab.id))
|
||||
.flatMap((tab) => collectPersistedTerminalPtyIds(session, tab))
|
||||
)
|
||||
const quickCommandLabelByTerminalId = new Map(
|
||||
@@ -3980,11 +3981,14 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
|
||||
.filter((tab) => {
|
||||
// Why: canonical mounts win PTY ownership over stale legacy duplicates.
|
||||
// Why: old web-client mirrors could persist host surface ids with "::"; makePaneKey reserves ":" as its separator.
|
||||
const persistedPtyIds = collectPersistedTerminalPtyIds(session, tab)
|
||||
// Why: drop only rows the canonical mount fully subsumes; a split tab with one independent pane still owns a PTY nothing else can reattach.
|
||||
// Why: the length guard keeps `every` from swallowing PTY-less rows, which own nothing to duplicate.
|
||||
const isPureDuplicate =
|
||||
persistedPtyIds.length > 0 &&
|
||||
persistedPtyIds.every((ptyId) => canonicalPtyIds.has(ptyId))
|
||||
return (
|
||||
(canonicalTerminalIds.has(tab.id) ||
|
||||
!collectPersistedTerminalPtyIds(session, tab).some((ptyId) =>
|
||||
canonicalPtyIds.has(ptyId)
|
||||
)) &&
|
||||
(canonicalTerminalIds.has(tab.id) || !isPureDuplicate) &&
|
||||
isValidTerminalTabId(tab.id)
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user