Editor files that share a path across two hosts: the tab lookup, the load report and the phone close match the right file (dark, layout-owner build only) (#30569)

* fix(layout): two owners' files at one path keep their own host, load report entry and phone retire (layout-owner build only)

The bare-path tab's file lookup took the first record of its path, the load report keyed file
records by path alone, and a headless host's phone retire found and removed records by path.

* fix(layout): a path-id preview tab's flag stays on its own owner's record, and a stale phone id retires nothing (layout-owner build only)
This commit is contained in:
Jinwoo Hong
2026-10-19 01:48:21 -04:00
committed by GitHub
parent e8d7da029c
commit 6c40f9190d
11 changed files with 235 additions and 50 deletions
@@ -95,3 +95,81 @@ describe('headless editor tabs under an owned id', () => {
])
})
})
const ENV = 'env-1'
/** A runtime-environment and a local file at one path: the runtime one listed first and owned. */
function twoOwnerSession(): WorkspaceSessionState {
const base = ownedSession()
const file = { filePath: FILE, relativePath: 'notes.ts', worktreeId: WT, language: 'typescript' }
const tab = base.unifiedTabs![WT]![0]!
return {
...base,
openFilesByWorktree: {
[WT]: [{ ...file, runtimeEnvironmentId: ENV, dirtyDraftContent: 'runtime draft' }, file]
},
unifiedTabs: {
[WT]: [
{ ...tab, id: 'tab-local', entityId: FILE, color: null, isPinned: false },
{ ...tab, id: 'tab-runtime', entityId: ownedEditorFileId(FILE, WT, ENV) }
]
},
tabGroups: {
[WT]: [
{
id: 'group-1',
worktreeId: WT,
activeTabId: 'tab-local',
tabOrder: ['tab-local', 'tab-runtime']
}
]
},
activeTabIdByWorktree: { [WT]: 'tab-local' },
activeTabTypeByWorktree: { [WT]: 'editor' },
activeFileIdByWorktree: { [WT]: FILE }
}
}
describe('two owners of one path on a headless host, layout-owner build', () => {
it("lists each record under its own owner's tab", () => {
expect(buildHeadlessMobileSessionEditorTabs(WT, twoOwnerSession())).toEqual([
expect.objectContaining({ id: 'tab-runtime', isDirty: true, isActive: false }),
expect.objectContaining({ id: 'tab-local', isDirty: false, isActive: true })
])
})
it('retiring the runtime tab keeps the local record, its tab and the active file', () => {
const retired = retireHeadlessEditorTab(
twoOwnerSession(),
WT,
{ id: 'tab-runtime', filePath: FILE },
true
)
expect(retired?.openFilesByWorktree?.[WT]).toEqual([
twoOwnerSession().openFilesByWorktree?.[WT]?.[1]
])
expect(retired?.unifiedTabs?.[WT]?.map((tab) => tab.id)).toEqual(['tab-local'])
expect(retired?.activeFileIdByWorktree?.[WT]).toBe(FILE)
expect(retired?.activeTabIdByWorktree?.[WT]).toBe('tab-local')
})
it("retiring the local tab ignores the runtime record's draft and keeps that record", () => {
const retired = retireHeadlessEditorTab(twoOwnerSession(), WT, {
id: 'tab-local',
filePath: FILE
})
expect(retired?.openFilesByWorktree?.[WT]?.map((file) => file.runtimeEnvironmentId)).toEqual([
ENV
])
expect(retired?.unifiedTabs?.[WT]?.map((tab) => tab.id)).toEqual(['tab-runtime'])
expect(retired?.activeFileIdByWorktree?.[WT]).toBeNull()
})
it('retires nothing for a tab id neither owner is listed under', () => {
expect(
retireHeadlessEditorTab(twoOwnerSession(), WT, { id: 'tab-gone', filePath: FILE }, true)
).toBeNull()
})
})
@@ -9,7 +9,11 @@ import type {
RuntimeMobileSessionTabsSnapshot
} from '../../shared/runtime-types'
import type { Tab } from '../../shared/tab-types'
import { editorTabShowsFile } from '../../shared/workspace-layout/workspace-layout-editor-tab-file'
import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../shared/layout-owner-switch'
import {
editorTabNamesFile,
sameEditorFile
} from '../../shared/workspace-layout/workspace-layout-editor-tab-file'
import type {
PersistedOpenFile,
WorkspaceSessionState
@@ -17,16 +21,30 @@ import type {
export type HeadlessEditorTab = RuntimeMobileSessionMarkdownTab | RuntimeMobileSessionFileTab
function editorWorkspace(session: WorkspaceSessionState, worktreeId: string) {
return {
worktreeId,
tabs: session.unifiedTabs?.[worktreeId] ?? [],
editorFiles: session.openFilesByWorktree?.[worktreeId]
}
}
function editorUnifiedTab(
session: WorkspaceSessionState,
worktreeId: string,
file: PersistedOpenFile
): Tab | undefined {
return (session.unifiedTabs?.[worktreeId] ?? []).find(
(tab) => tab.contentType === 'editor' && editorTabShowsFile(tab.entityId, file, worktreeId)
const workspace = editorWorkspace(session, worktreeId)
return workspace.tabs.find(
(tab) => tab.contentType === 'editor' && editorTabNamesFile(workspace, tab.entityId, file)
)
}
/** A file's phone tab id: its unified tab's, else its path. */
function listedTabId(unifiedTab: Tab | undefined, file: PersistedOpenFile): string {
return unifiedTab?.id ?? file.filePath
}
function editorTab(
session: WorkspaceSessionState,
worktreeId: string,
@@ -35,8 +53,7 @@ function editorTab(
const unifiedTab = editorUnifiedTab(session, worktreeId, file)
const activeFileId = session.activeFileIdByWorktree?.[worktreeId]
const common = {
// A file's id is its path, which is also the unified tab's entity.
id: unifiedTab?.id ?? file.filePath,
id: listedTabId(unifiedTab, file),
title: file.relativePath.split(/[\\/]/).pop() || file.relativePath || 'File',
filePath: file.filePath,
relativePath: file.relativePath,
@@ -44,7 +61,7 @@ function editorTab(
isActive:
session.activeTabTypeByWorktree?.[worktreeId] === 'editor' &&
!!activeFileId &&
editorTabShowsFile(activeFileId, file, worktreeId),
editorTabNamesFile(editorWorkspace(session, worktreeId), activeFileId, file),
color: unifiedTab?.color ?? null,
isPinned: unifiedTab?.isPinned === true
}
@@ -74,6 +91,23 @@ export function buildHeadlessMobileSessionEditorTabs(
)
}
/** The record the phone's tab names: two owners can hold one path, so by the id it was listed under. */
function listedFile(
files: readonly PersistedOpenFile[],
session: WorkspaceSessionState,
worktreeId: string,
tab: Pick<HeadlessEditorTab, 'id' | 'filePath'>
): PersistedOpenFile | undefined {
const samePath = files.filter((candidate) => candidate.filePath === tab.filePath)
// Why no fallback for two: a stale id would retire the other owner's record.
return samePath.length < 2
? samePath[0]
: samePath.find(
(candidate) =>
listedTabId(editorUnifiedTab(session, worktreeId, candidate), candidate) === tab.id
)
}
/** The session without one persisted editor tab, or null when it holds no such tab. */
export function retireHeadlessEditorTab(
session: WorkspaceSessionState,
@@ -82,7 +116,9 @@ export function retireHeadlessEditorTab(
force = false
): WorkspaceSessionState | null {
const files = session.openFilesByWorktree?.[worktreeId] ?? []
const file = files.find((candidate) => candidate.filePath === tab.filePath)
const file = ORCA_LAYOUT_OWNER_IS_MAIN
? listedFile(files, session, worktreeId, tab)
: files.find((candidate) => candidate.filePath === tab.filePath)
if (!file) {
return null
}
@@ -100,12 +136,13 @@ export function retireHeadlessEditorTab(
...(group.recentTabIds ? { recentTabIds: withoutTab(group.recentTabIds) } : {})
}))
const activeFileId = session.activeFileIdByWorktree?.[worktreeId]
const wasActiveFile = !!activeFileId && editorTabShowsFile(activeFileId, file, worktreeId)
const wasActiveFile =
!!activeFileId && editorTabNamesFile(editorWorkspace(session, worktreeId), activeFileId, file)
return {
...session,
openFilesByWorktree: {
...session.openFilesByWorktree,
[worktreeId]: files.filter((file) => file.filePath !== tab.filePath)
[worktreeId]: files.filter((candidate) => !sameEditorFile(candidate, file))
},
unifiedTabs: {
...session.unifiedTabs,
@@ -9,8 +9,11 @@ import type { PersistedOpenFile, WorkspaceSessionState } from '../workspace-sess
import { applyLayoutCommand } from './workspace-layout-commands'
import type { CommandOf } from './workspace-layout-command-types'
import { build, testContext } from './workspace-layout-command.test-fixture'
import { editorTabFile } from './workspace-layout-editor-tab-file'
import { loadWorkspaceLayout } from './workspace-layout-load'
import { diffStoredSession } from './workspace-layout-load-report'
import type { LayoutEditorFile, WorkspaceLayoutModel } from './workspace-layout-model'
import { publishWorkspaceLayout } from './workspace-layout-published'
import { patchStoredSession } from './workspace-layout-record-patch'
import { saveWorkspaceLayout } from './workspace-layout-save'
import { addWorkspace, emptySession, GIT_KEY } from './workspace-layout-session.test-fixture'
@@ -189,6 +192,67 @@ describe('editor file records keyed by path and owner, layout-owner build', () =
expect(patched.openFilesByWorktree?.[FLOATING]).toEqual([local, runtime])
})
it('a path-id tab names the local record when the runtime record is listed first', () => {
const { model } = build(testContext(), [
open(GIT_KEY, runtimeId(GIT_KEY), RUNTIME_FILE),
open(GIT_KEY, PATH, LOCAL_FILE)
])
const workspace = model.workspaces[GIT_KEY]!
const hostOf = (entityId: string) =>
publishWorkspaceLayout(workspace, LOCAL_EXECUTION_HOST_ID).tabs.find(
(tab) => tab.entityId === entityId
)?.executionHostId
expect(owners(model, GIT_KEY)).toEqual([ENV, 'local'])
expect(editorTabFile(workspace, PATH)?.runtimeEnvironmentId).toBeUndefined()
expect(hostOf(PATH)).toBe(LOCAL_EXECUTION_HOST_ID)
expect(hostOf(runtimeId(GIT_KEY))).toBe(`runtime:${ENV}`)
})
it("the load report sees a change to one owner's record of a shared path", () => {
const local = { ...LOCAL_FILE, worktreeId: FLOATING }
const runtime = { ...RUNTIME_FILE, worktreeId: FLOATING }
const session = (files: PersistedOpenFile[]): WorkspaceSessionState => ({
...emptySession(),
openFilesByWorktree: { [FLOATING]: files }
})
const changes = diffStoredSession(session([local, runtime]), session([runtime]))
expect(changes.filter((change) => change.table === 'file')).toEqual([
expect.objectContaining({ field: '*', before: local, after: undefined })
])
})
it("a reload keeps a path-id preview tab's flag on its own owner's record only", () => {
const stored: WorkspaceSessionState = addWorkspace(emptySession(), GIT_KEY, [
{
id: 'group-1',
tabs: [
{ id: PATH, kind: 'editor', entry: { isPreview: true } },
{ id: runtimeId(GIT_KEY), kind: 'editor' }
]
}
])
stored.openFilesByWorktree = {
[GIT_KEY]: [
{ ...RUNTIME_FILE, worktreeId: GIT_KEY },
{ ...LOCAL_FILE, worktreeId: GIT_KEY, isPreview: true }
]
}
let id = 0
const loaded = loadWorkspaceLayout(LOCAL_EXECUTION_HOST_ID, stored, {
mintId: () => `minted-${++id}`,
mintLeafId: () => `00000000-0000-4000-8000-${String(++id).padStart(12, '0')}`
})
const tabs = loaded.layout.workspaces[GIT_KEY]!.tabs
expect(tabs.map((tab) => tab.isPreview)).toEqual([true, undefined])
expect(saveWorkspaceLayout(loaded).openFilesByWorktree?.[GIT_KEY]).toEqual(
stored.openFilesByWorktree[GIT_KEY]
)
})
it('match by path alone when the switch is off, as a headless host does today', () => {
mocks.layoutOwnerIsMain = false
const { model } = bothOpen(FLOATING, localId(FLOATING))
@@ -42,21 +42,35 @@ export function holdsEditorFileId(
)
}
export function editorTabFile<F extends FileOwner>(
workspace: { worktreeId: string; editorFiles?: readonly F[] },
entityId: string
): F | undefined {
return workspace.editorFiles?.find((file) =>
editorTabShowsFile(entityId, file, workspace.worktreeId)
)
}
type EditorWorkspace<F> = {
worktreeId: string
tabs: readonly { entityId: string }[]
editorFiles?: readonly F[]
}
/** The record an editor tab shows; in a layout-owner build a bare path skips owned-id tabs' records. */
export function editorTabFile<F extends FileOwner>(
workspace: EditorWorkspace<F>,
entityId: string
): F | undefined {
return ORCA_LAYOUT_OWNER_IS_MAIN
? editorTabFiles(workspace, entityId)[0]
: workspace.editorFiles?.find((file) =>
editorTabShowsFile(entityId, file, workspace.worktreeId)
)
}
/** Whether an editor file id names this record, as `editorTabFile` resolves it. */
export function editorTabNamesFile<F extends FileOwner>(
workspace: EditorWorkspace<F>,
entityId: string,
file: F
): boolean {
return ORCA_LAYOUT_OWNER_IS_MAIN
? editorTabFile(workspace, entityId) === file
: editorTabShowsFile(entityId, file, workspace.worktreeId)
}
/**
* The records an editor tab shows. The window gives a path's bare id to one owner only, so a bare
* id that matches several records names those no owned-id tab claims.
@@ -6,7 +6,7 @@ import {
EDITOR_DRAFT_FIELDS
} from './workspace-layout-beside'
import { childRecord, omitStoredFields, pickStoredFields } from './stored-record-fields'
import { editorFileKey, editorTabShowsFile } from './workspace-layout-editor-tab-file'
import { editorFileKey, editorTabNamesFile } from './workspace-layout-editor-tab-file'
import type { WorkspaceLoadArgs } from './workspace-layout-load-types'
import type { LayoutBrowserTab, LayoutEditorFile, LayoutTab } from './workspace-layout-model'
@@ -51,13 +51,14 @@ export function loadEditorFiles(
if (!files) {
return undefined
}
const workspace = { worktreeId, tabs, editorFiles: files }
return files.map((file) => {
const draft = pickStoredFields(file, EDITOR_DRAFT_FIELDS)
if (Object.keys(draft).length > 0) {
childRecord(view.editorDrafts, key)[editorFileKey(file)] = draft
}
const fileTabs = tabs.filter(
(tab) => tab.kind === 'editor' && editorTabShowsFile(tab.entityId, file, worktreeId)
(tab) => tab.kind === 'editor' && editorTabNamesFile(workspace, tab.entityId, file)
)
// The file's copy is true when any of its tabs is preview (split groups share one file).
const demoted = file.isPreview === true ? [] : fileTabs.filter((tab) => tab.isPreview)
@@ -3,6 +3,7 @@
import type { WorkspaceSessionState } from '../workspace-session-state-types'
import type { DiskTable } from './workspace-layout-disk-fields'
import { editorFileKey } from './workspace-layout-editor-tab-file'
import { collectLayoutLeafIdsInOrder } from './terminal-pane-tree'
import { paneKeyOf } from './workspace-layout-model'
@@ -78,7 +79,7 @@ export function diskRecords(session: WorkspaceSessionState): DiskRecords {
groups.forEach((group) => records.group.set(`${key}|${group.id}`, group))
}
for (const [key, files] of Object.entries(session.openFilesByWorktree ?? {})) {
files.forEach((file) => records.file.set(`${key}|${file.filePath}`, file))
files.forEach((file) => records.file.set(`${key}|${editorFileKey(file)}`, file))
}
for (const [key, tabs] of Object.entries(session.browserTabsByWorktree ?? {})) {
tabs.forEach((tab) => records.browser.set(`${key}|${tab.id}`, tab))
@@ -105,14 +105,9 @@ describe('editor tabs under an owned id, layout-owner build', () => {
const { layout } = load(session)
const workspace = layout.workspaces[GIT_KEY]!
expect(
tabExecutionHostId(
workspace.tabs[0]!,
workspace.editorFiles,
LOCAL_EXECUTION_HOST_ID,
workspace.worktreeId
)
).toBe('runtime:env-1')
expect(tabExecutionHostId(workspace.tabs[0]!, workspace, LOCAL_EXECUTION_HOST_ID)).toBe(
'runtime:env-1'
)
const closed = applyLayoutCommand(
layout,
{ type: 'closeTabs', workspace: GIT_KEY, tabIds: [fileId] },
@@ -96,12 +96,7 @@ function publishTab(
id: tab.id,
kind: tab.kind,
entityId: tab.entityId,
executionHostId: tabExecutionHostId(
tab,
workspace.editorFiles,
partition,
workspace.worktreeId
),
executionHostId: tabExecutionHostId(tab, workspace, partition),
createdAt: tab.createdAt,
customTitle: tab.customTitle,
color: tab.color,
@@ -8,10 +8,10 @@ import { pickStoredFields } from './stored-record-fields'
import { collectLayoutLeafIdsInOrder } from './terminal-pane-tree'
import type { DesktopLayoutView, LayoutContentFacts } from './workspace-layout-beside'
import type {
LayoutEditorFile,
LayoutLeaf,
LayoutTab,
LayoutTerminalTab
LayoutTerminalTab,
WorkspaceLayout
} from './workspace-layout-model'
import { tabExecutionHostId } from './workspace-layout-tab-host'
@@ -27,7 +27,7 @@ export type WorkspaceSaveScope = {
workspaceKey: string
worktreeId: string
hostId: ExecutionHostId
editorFiles: readonly LayoutEditorFile[] | undefined
workspace: Pick<WorkspaceLayout, 'worktreeId' | 'tabs' | 'editorFiles'>
leaves: Readonly<Record<string, LayoutLeaf>>
facts: LayoutContentFacts
view: DesktopLayoutView
@@ -94,7 +94,7 @@ export function saveTabBarEntry(
entityId: tab.entityId,
groupId: placement.groupId,
worktreeId: scope.worktreeId,
executionHostId: tabExecutionHostId(tab, scope.editorFiles, scope.hostId, scope.worktreeId),
executionHostId: tabExecutionHostId(tab, scope.workspace, scope.hostId),
contentType: tab.kind,
label,
...(tab.generatedTitle !== undefined ? { generatedLabel: tab.generatedTitle } : {}),
@@ -15,7 +15,7 @@ import {
FACT_SESSION_FIELDS,
VIEW_SESSION_FIELDS
} from './workspace-layout-beside'
import { editorFileKey, editorTabShowsFile } from './workspace-layout-editor-tab-file'
import { editorFileKey, editorTabNamesFile } from './workspace-layout-editor-tab-file'
import type { LoadedWorkspaceLayout } from './workspace-layout-load-types'
import { collectLayoutLeafIdsInOrder } from './terminal-pane-tree'
import { paneKeyOf, tabsInOrder, type WorkspaceLayout } from './workspace-layout-model'
@@ -43,7 +43,7 @@ function saveWorkspace(
workspaceKey: key,
worktreeId,
hostId: loaded.layout.hostId,
editorFiles: workspace.editorFiles,
workspace,
leaves: workspace.leaves ?? {},
facts,
view
@@ -104,7 +104,7 @@ function saveWorkspace(
session.openFilesByWorktree[key] = workspace.editorFiles.map((file): PersistedOpenFile => ({
...file,
worktreeId,
...(previewTabs.some((tab) => editorTabShowsFile(tab.entityId, file, worktreeId))
...(previewTabs.some((tab) => editorTabNamesFile(workspace, tab.entityId, file))
? { isPreview: true }
: {}),
...view.editorDrafts[key]?.[editorFileKey(file)]
@@ -8,7 +8,7 @@ import {
} from '../execution-host'
import type { TabContentType } from '../tab-types'
import type { PersistedOpenFile } from '../workspace-session-state-types'
import { editorTabShowsFile } from './workspace-layout-editor-tab-file'
import { editorTabFile } from './workspace-layout-editor-tab-file'
type FileOwner = Pick<
PersistedOpenFile,
@@ -18,14 +18,14 @@ type FileOwner = Pick<
/** An editor tab follows its file's owner, as the window stamps it; every other tab its partition. */
export function tabExecutionHostId(
tab: { kind: TabContentType; entityId: string },
files: readonly FileOwner[] | undefined,
hostId: ExecutionHostId,
worktreeId: string
workspace: {
worktreeId: string
tabs: readonly { entityId: string }[]
editorFiles?: readonly FileOwner[]
},
hostId: ExecutionHostId
): ExecutionHostId {
const file =
tab.kind === 'editor'
? files?.find((entry) => editorTabShowsFile(tab.entityId, entry, worktreeId))
: undefined
const file = tab.kind === 'editor' ? editorTabFile(workspace, tab.entityId) : undefined
if (file?.externalSshTargetId) {
return toSshExecutionHostId(file.externalSshTargetId)
}