fix: close mirrored editor tabs on the host so they stop reopening (#5975)

* fix: close mirrored editor tabs on the host so they stop reopening

On web and mobile companions, editor file tabs are mirrored from the
host's runtime-session snapshot, which the host derives from its
authoritative `openFiles`. Closing a mirrored tab only removed it
locally, so the next snapshot re-mirrored the still-open host file and
the tab immediately reopened.

Route the close to the host from the single chokepoint every editor
close funnels through (`store.closeFile`): when the file is mirrored and
a web runtime session is active, send `session.tabs.close` for the host
tab id. The RPC's recorded close-intent suppresses re-mirroring until the
host snapshot catches up, so the local removal is not undone. No-op for
the host's own (non-mirrored) files.

Also fix the desktop `ui:closeSessionTab` handler so a companion-driven
editor close goes through `closeFile` (clears `openFiles`) instead of
`closeUnifiedTab` (tab strip only), which had the same re-mirror bug for
mobile-originated closes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Fix mirrored editor close import cycle

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
gsxdsm
2026-06-22 14:51:43 -07:00
committed by GitHub
co-authored by Claude Opus 4.8 Orca Jinwoo-H
parent 10789669ec
commit b58478eae0
8 changed files with 367 additions and 19 deletions
@@ -8,9 +8,9 @@ import {
describe('acceptSessionSnapshot', () => {
it('accepts a newer version from the same publisher and advances the floor', () => {
const marker: AppliedSnapshotMarker = { epoch: 'renderer:a', version: 5 }
expect(acceptSessionSnapshot({ publicationEpoch: 'renderer:a', snapshotVersion: 6 }, marker)).toBe(
true
)
expect(
acceptSessionSnapshot({ publicationEpoch: 'renderer:a', snapshotVersion: 6 }, marker)
).toBe(true)
expect(marker).toEqual({ epoch: 'renderer:a', version: 6 })
})
@@ -806,12 +806,8 @@ describe('TabBar PowerShell launch wiring', () => {
onTogglePaneExpand: () => {}
})
expect(
findDropdownMenuItemByText(expandNode(element), 'New Terminal: PowerShell')
).toBeNull()
expect(
findDropdownMenuItemByText(expandNode(element), 'New Terminal: CMD Prompt')
).toBeNull()
expect(findDropdownMenuItemByText(expandNode(element), 'New Terminal: PowerShell')).toBeNull()
expect(findDropdownMenuItemByText(expandNode(element), 'New Terminal: CMD Prompt')).toBeNull()
expect(findDropdownMenuItemByText(expandNode(element), 'New Terminal: WSL')).toBeNull()
expect(findDropdownMenuItemByText(expandNode(element), 'New Terminal')).not.toBeNull()
})
+66 -1
View File
@@ -1929,15 +1929,18 @@ describe('useIpcEvents browser tab close routing', () => {
}) => void
type CloseActiveTabListener = () => void
type CloseTerminalListener = (data: { tabId: string; paneRuntimeId?: number | null }) => void
type CloseSessionTabListener = (data: { tabId: string; worktreeId: string }) => void
async function useIpcEventsForCloseRouting({
closeActiveTabListenerRef,
closeSessionTabListenerRef,
closeTerminalListenerRef,
getState,
requestTabCloseListenerRef,
replyTabClose = vi.fn()
}: {
closeActiveTabListenerRef?: { current: CloseActiveTabListener | null }
closeSessionTabListenerRef?: { current: CloseSessionTabListener | null }
closeTerminalListenerRef?: { current: CloseTerminalListener | null }
getState: () => Record<string, unknown>
requestTabCloseListenerRef?: { current: RequestTabCloseListener | null }
@@ -1987,6 +1990,7 @@ describe('useIpcEvents browser tab close routing', () => {
activeBrowserTabIdByWorktree: { 'wt-1': 'workspace-1' },
browserTabsByWorktree: { 'wt-1': [{ id: 'workspace-1' }] },
browserPagesByWorkspace: {},
openFiles: [],
unifiedTabsByWorktree: {},
closeBrowserTab: vi.fn(),
closeBrowserPage: vi.fn(),
@@ -2053,7 +2057,12 @@ describe('useIpcEvents browser tab close routing', () => {
onRenameTerminal: () => () => {},
onFocusTerminal: () => () => {},
onFocusEditorTab: () => () => {},
onCloseSessionTab: () => () => {},
onCloseSessionTab: (listener: CloseSessionTabListener) => {
if (closeSessionTabListenerRef) {
closeSessionTabListenerRef.current = listener
}
return () => {}
},
onMoveSessionTab: () => () => {},
onOpenFileFromMobile: () => () => {},
onOpenDiffFromMobile: () => () => {},
@@ -2140,6 +2149,62 @@ describe('useIpcEvents browser tab close routing', () => {
registerIpcEvents()
}
it('removes the file from openFiles when a companion closes an editor session tab', async () => {
const closeSessionTabListenerRef: { current: CloseSessionTabListener | null } = {
current: null
}
const closeFile = vi.fn()
const closeUnifiedTab = vi.fn()
await useIpcEventsForCloseRouting({
closeSessionTabListenerRef,
getState: () => ({
closeFile,
closeUnifiedTab,
browserTabsByWorktree: {},
unifiedTabsByWorktree: {
'wt-1': [{ id: 'host-tab-1', entityId: 'file-1', contentType: 'editor', isPinned: false }]
}
})
})
closeSessionTabListenerRef.current?.({ tabId: 'host-tab-1', worktreeId: 'wt-1' })
// Why: closeUnifiedTab alone would leave the file in openFiles, which the host
// republishes — so the editor close must go through closeFile.
expect(closeFile).toHaveBeenCalledWith('file-1')
expect(closeUnifiedTab).not.toHaveBeenCalled()
})
it('keeps closeUnifiedTab for a non-editor session tab closed by a companion', async () => {
const closeSessionTabListenerRef: { current: CloseSessionTabListener | null } = {
current: null
}
const closeFile = vi.fn()
const closeUnifiedTab = vi.fn()
await useIpcEventsForCloseRouting({
closeSessionTabListenerRef,
getState: () => ({
closeFile,
closeUnifiedTab,
browserTabsByWorktree: {},
unifiedTabsByWorktree: {
'wt-1': [
{ id: 'sim-tab-1', entityId: 'sim-1', contentType: 'simulator', isPinned: false }
]
}
})
})
closeSessionTabListenerRef.current?.({ tabId: 'sim-tab-1', worktreeId: 'wt-1' })
// Why: only editor tabs need the closeFile (openFiles) path; other content types
// must keep closeUnifiedTab so the editor-only routing stays scoped.
expect(closeUnifiedTab).toHaveBeenCalledWith('sim-tab-1')
expect(closeFile).not.toHaveBeenCalled()
})
it('delegates terminal close IPC without a pane id to the shared terminal close flow', async () => {
const closeTerminalListenerRef: { current: CloseTerminalListener | null } = { current: null }
@@ -98,11 +98,12 @@ describe('getExplicitRuntimeEnvironmentIdForWorktree', () => {
'runtime-repo::wt-local-override'
)
).toBeNull()
expect(getRuntimeEnvironmentIdForWorktree(hostOverrideState, 'runtime-repo::wt-local-override'))
.toBeNull()
expect(getExecutionHostIdForWorktree(hostOverrideState, 'runtime-repo::wt-local-override')).toBe(
'local'
)
expect(
getRuntimeEnvironmentIdForWorktree(hostOverrideState, 'runtime-repo::wt-local-override')
).toBeNull()
expect(
getExecutionHostIdForWorktree(hostOverrideState, 'runtime-repo::wt-local-override')
).toBe('local')
expect(
getExplicitRuntimeEnvironmentIdForWorktree(
hostOverrideState,
@@ -0,0 +1,72 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const closeWebRuntimeSessionTabMock = vi.fn()
const getRuntimeEnvironmentIdForWorktreeMock = vi.fn()
vi.mock('./web-runtime-session', () => ({
closeWebRuntimeSessionTab: (args: unknown) => closeWebRuntimeSessionTabMock(args)
}))
vi.mock('@/lib/worktree-runtime-owner', () => ({
getRuntimeEnvironmentIdForWorktree: (...args: unknown[]) =>
getRuntimeEnvironmentIdForWorktreeMock(...args)
}))
import {
notifyHostOfMirroredEditorClose,
type MirroredEditorCloseState
} from './close-mirrored-editor-tab'
function buildState(overrides: Partial<MirroredEditorCloseState> = {}): MirroredEditorCloseState {
return {
openFiles: [{ id: 'file-1', worktreeId: 'wt-1', mirroredFromRuntimeSession: true }],
unifiedTabsByWorktree: {
'wt-1': [{ id: 'host-tab-1', entityId: 'file-1', contentType: 'editor' }]
},
...overrides
} as unknown as MirroredEditorCloseState
}
describe('notifyHostOfMirroredEditorClose', () => {
beforeEach(() => {
closeWebRuntimeSessionTabMock.mockReset()
getRuntimeEnvironmentIdForWorktreeMock.mockReset()
getRuntimeEnvironmentIdForWorktreeMock.mockReturnValue('env-1')
})
it('closes the mirrored editor tab on the host using the host tab id', async () => {
const handled = notifyHostOfMirroredEditorClose(buildState(), 'wt-1', 'file-1')
expect(handled).toBe(true)
await vi.waitFor(() => {
expect(closeWebRuntimeSessionTabMock).toHaveBeenCalled()
})
expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledWith({
worktreeId: 'wt-1',
tabId: 'host-tab-1',
environmentId: 'env-1'
})
})
it('does not route locally-opened (non-mirrored) files to the host', () => {
const state = buildState({
openFiles: [
{ id: 'file-1', worktreeId: 'wt-1' }
] as unknown as MirroredEditorCloseState['openFiles']
})
const handled = notifyHostOfMirroredEditorClose(state, 'wt-1', 'file-1')
expect(handled).toBe(false)
expect(closeWebRuntimeSessionTabMock).not.toHaveBeenCalled()
})
it('does nothing when no web runtime session is active', () => {
getRuntimeEnvironmentIdForWorktreeMock.mockReturnValue(null)
const handled = notifyHostOfMirroredEditorClose(buildState(), 'wt-1', 'file-1')
expect(handled).toBe(false)
expect(closeWebRuntimeSessionTabMock).not.toHaveBeenCalled()
})
})
@@ -0,0 +1,54 @@
import {
getRuntimeEnvironmentIdForWorktree,
type WorktreeRuntimeOwnerState
} from '@/lib/worktree-runtime-owner'
import type { OpenFile } from '@/store/slices/editor'
import type { Tab } from '../../../shared/types'
export type MirroredEditorCloseState = WorktreeRuntimeOwnerState & {
openFiles: readonly OpenFile[]
unifiedTabsByWorktree: Record<string, Tab[]>
}
// Why: an editor file mirrored from a runtime session is owned by the host. The
// host republishes its open files to companions, so removing the tab only locally
// is undone by the next snapshot. Tell the host to close its own tab; the close
// intent recorded by the RPC suppresses re-mirroring until the snapshot catches up.
// Side-effecting only — callers still run their normal local close. No-op (returns
// false) for non-mirrored files, so the host's own closes are untouched.
export function notifyHostOfMirroredEditorClose(
state: MirroredEditorCloseState,
worktreeId: string | null | undefined,
fileId: string
): boolean {
if (!worktreeId) {
return false
}
const file = state.openFiles.find((candidate) => candidate.id === fileId)
if (!file?.mirroredFromRuntimeSession) {
return false
}
const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree(state, worktreeId)
if (!runtimeEnvironmentId?.trim()) {
return false
}
// Why: a mirrored editor unified tab carries the host's tab id as `id` and the
// local file id as `entityId`, and the host close RPC resolves editor tabs by id.
const unifiedTab = (state.unifiedTabsByWorktree[worktreeId] ?? []).find(
(tab) => tab.contentType === 'editor' && tab.entityId === fileId
)
if (!unifiedTab) {
return false
}
// Why: this helper is imported by the editor slice during store creation.
// Importing web-runtime-session eagerly would import the store back and can
// trip cyclic initialization in full-suite test/import order.
void import('./web-runtime-session').then(({ closeWebRuntimeSessionTab }) =>
closeWebRuntimeSessionTab({
worktreeId,
tabId: unifiedTab.id,
environmentId: runtimeEnvironmentId
})
)
return true
}
+145 -1
View File
@@ -11,7 +11,7 @@ import {
} from '../../runtime/runtime-compatibility-test-fixture'
import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client'
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
import type { GitStatusEntry } from '../../../../shared/types'
import type { GitStatusEntry, Tab } from '../../../../shared/types'
const { toastErrorMock } = vi.hoisted(() => ({
toastErrorMock: vi.fn()
@@ -26,6 +26,14 @@ vi.mock('@/lib/http-link-routing', () => ({
openHttpLink: openHttpLinkMock
}))
const { notifyHostOfMirroredEditorCloseMock } = vi.hoisted(() => ({
notifyHostOfMirroredEditorCloseMock: vi.fn()
}))
vi.mock('@/runtime/close-mirrored-editor-tab', () => ({
notifyHostOfMirroredEditorClose: (...args: unknown[]) =>
notifyHostOfMirroredEditorCloseMock(...args)
}))
function createEditorStore(): StoreApi<AppState> {
// Only the editor slice + activeWorktreeId are needed for these tests.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -68,6 +76,21 @@ function ownedEditorFileId(
return `editor:${encodeURIComponent(worktreeId)}:${encodeURIComponent(runtimeKey)}:${encodeURIComponent(filePath)}`
}
function mirroredEditorUnifiedTab(id: string, entityId: string, worktreeId: string): Tab {
return {
id,
entityId,
worktreeId,
groupId: `${worktreeId}:group`,
contentType: 'editor',
label: entityId,
customLabel: null,
color: null,
sortOrder: 0,
createdAt: 0
}
}
describe('createEditorSlice right sidebar state', () => {
it('does not record markdown-file-created when opening an existing markdown file', () => {
const store = createEditorStore()
@@ -4020,3 +4043,124 @@ describe('createEditorSlice activateMarkdownLink', () => {
expect(store.getState().pendingEditorReveal?.line).toBe(3)
})
})
describe('closeFile host mirroring', () => {
beforeEach(() => {
notifyHostOfMirroredEditorCloseMock.mockReset()
})
it('routes every close through the host-mirror notifier and still removes the file locally', () => {
const store = createEditorTabsStore()
store.getState().openFile({
filePath: '/repo/a.ts',
relativePath: 'a.ts',
worktreeId: 'wt-1',
language: 'typescript',
mode: 'edit'
})
const fileId = store.getState().openFiles[0]!.id
store.getState().closeFile(fileId)
// Why: closeFile is the single chokepoint, so a mirrored tab closed via any
// surface (tab strip, bulk close, save/discard) reaches the host. The notifier
// itself no-ops for non-mirrored files; here we assert the wiring + local close.
expect(notifyHostOfMirroredEditorCloseMock).toHaveBeenCalledWith(
expect.anything(),
'wt-1',
fileId
)
expect(store.getState().openFiles).toHaveLength(0)
})
it('notifies the host for mirrored editors removed by close all in the active worktree', () => {
const store = createEditorTabsStore()
store.getState().openFile({
filePath: '/repo/a.ts',
relativePath: 'a.ts',
worktreeId: 'wt-1',
language: 'typescript',
mode: 'edit',
mirroredFromRuntimeSession: true
})
store.getState().openFile({
filePath: '/other/b.ts',
relativePath: 'b.ts',
worktreeId: 'wt-2',
language: 'typescript',
mode: 'edit',
mirroredFromRuntimeSession: true
})
store.setState({
unifiedTabsByWorktree: {
'wt-1': [mirroredEditorUnifiedTab('host-tab-a', '/repo/a.ts', 'wt-1')],
'wt-2': [mirroredEditorUnifiedTab('host-tab-b', '/other/b.ts', 'wt-2')]
},
tabBarOrderByWorktree: {
'wt-1': ['host-tab-a'],
'wt-2': ['host-tab-b']
}
} as Partial<AppState>)
store.getState().closeAllFiles()
// Why: closeAllFiles mutates openFiles directly instead of calling closeFile,
// so it must still run the host close hook for every removed mirrored editor.
expect(notifyHostOfMirroredEditorCloseMock).toHaveBeenCalledTimes(1)
expect(notifyHostOfMirroredEditorCloseMock).toHaveBeenCalledWith(
expect.anything(),
'wt-1',
'/repo/a.ts'
)
expect(store.getState().openFiles).toHaveLength(1)
expect(store.getState().openFiles[0]?.id).toBe('/other/b.ts')
expect(store.getState().tabBarOrderByWorktree['wt-1']).toEqual([])
expect(store.getState().tabBarOrderByWorktree['wt-2']).toEqual(['host-tab-b'])
})
it('notifies the host for every mirrored editor when close all has no active worktree', () => {
const store = createEditorTabsStore()
store.setState({ activeWorktreeId: null })
store.getState().openFile({
filePath: '/repo/a.ts',
relativePath: 'a.ts',
worktreeId: 'wt-1',
language: 'typescript',
mode: 'edit',
mirroredFromRuntimeSession: true
})
store.getState().openFile({
filePath: '/other/b.ts',
relativePath: 'b.ts',
worktreeId: 'wt-2',
language: 'typescript',
mode: 'edit',
mirroredFromRuntimeSession: true
})
store.setState({
unifiedTabsByWorktree: {
'wt-1': [mirroredEditorUnifiedTab('host-tab-a', '/repo/a.ts', 'wt-1')],
'wt-2': [mirroredEditorUnifiedTab('host-tab-b', '/other/b.ts', 'wt-2')]
},
tabBarOrderByWorktree: {
'wt-1': ['host-tab-a'],
'wt-2': ['host-tab-b']
}
} as Partial<AppState>)
store.getState().closeAllFiles()
expect(notifyHostOfMirroredEditorCloseMock).toHaveBeenCalledTimes(2)
expect(notifyHostOfMirroredEditorCloseMock).toHaveBeenCalledWith(
expect.anything(),
'wt-1',
'/repo/a.ts'
)
expect(notifyHostOfMirroredEditorCloseMock).toHaveBeenCalledWith(
expect.anything(),
'wt-2',
'/other/b.ts'
)
expect(store.getState().openFiles).toHaveLength(0)
})
})
+19 -3
View File
@@ -58,6 +58,7 @@ import {
statRuntimePath
} from '@/runtime/runtime-file-client'
import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client'
import { notifyHostOfMirroredEditorClose } from '@/runtime/close-mirrored-editor-tab'
import { findWorktreeById, getRepoIdFromWorktreeId } from './worktree-helpers'
import { createUntitledMarkdownFileWithTemplateSelection } from '@/lib/create-untitled-markdown'
import { extractIpcErrorMessage } from '@/lib/ipc-error'
@@ -1912,6 +1913,12 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
const hasDraft = !!get().editorDrafts[fileId]
const shouldDeleteFromDisk = shouldDeleteUntouchedUntitledFile(preClose, hasDraft)
// Why: closeFile is the single chokepoint every editor close funnels through
// (tab strips, bulk close, save/discard, floating panel). Mirrored tabs are
// host-owned, so the host must close its copy too or its next snapshot
// re-mirrors the file and the tab reopens. No-op for the host's own files.
notifyHostOfMirroredEditorClose(get(), preClose?.worktreeId, fileId)
set((s) => {
const closedFile = s.openFiles.find((f) => f.id === fileId)
const idx = s.openFiles.findIndex((f) => f.id === fileId)
@@ -2136,6 +2143,14 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
shouldDeleteUntouchedUntitledFile(f, !!state.editorDrafts[f.id]) &&
(!activeWorktreeId || f.worktreeId === activeWorktreeId)
)
const closingFiles = state.openFiles.filter(
(file) => !activeWorktreeId || file.worktreeId === activeWorktreeId
)
// Why: close-all bypasses closeFile's per-tab path, so mirrored host-owned
// editors must be notified here or the next host snapshot reopens them.
for (const file of closingFiles) {
notifyHostOfMirroredEditorClose(state, file.worktreeId, file.id)
}
const closingItemIds = Object.values(state.unifiedTabsByWorktree ?? {})
.flat()
@@ -2193,16 +2208,17 @@ export const createEditorSlice: StateCreator<AppState, [], [], EditorSlice> = (s
const shouldDeactivateWorktree =
browserTabsForWorktree.length === 0 && terminalTabsForWorktree.length === 0
// Why: remove all closed editor file IDs from tab bar order so stale
// entries don't cause position shifts on subsequent tab operations.
// Why: mirrored editor tabs use host tab ids in tab order, while local
// editor entries may still use file ids. Remove both close-all shapes.
const closedFileIds = new Set(
s.openFiles.filter((f) => f.worktreeId === activeWorktreeId).map((f) => f.id)
)
const closedTabOrderIds = new Set([...closedFileIds, ...closingItemIds])
const nextTabBarOrderByWorktree = s.tabBarOrderByWorktree
? {
...s.tabBarOrderByWorktree,
[activeWorktreeId]: (s.tabBarOrderByWorktree[activeWorktreeId] ?? []).filter(
(entryId) => !closedFileIds.has(entryId)
(entryId) => !closedTabOrderIds.has(entryId)
)
}
: s.tabBarOrderByWorktree