From ea60ec5d7cb9b542d2b5b3fd986acfc8861e4560 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 22 Oct 2026 05:26:12 -0400 Subject: [PATCH] Layout-owner builds: a phone's tab move is always the server's own; no ghost editor tabs after restart (#31925) * Layout groundwork (switched off): main derives agent tab titles from its own hook status With the layout-owner switch on, main gives agent tabs their generated titles from the hook-server status it owns, on the desktop, headless serve and orcad: one renameTabs per status batch, so one write and one frame. It keeps the window's rules (setting, custom and quick-command titles, first title wins, dispatch labels). The window stops sending generated titles for tabs main holds; a paired server's tab, or one main has no tab-bar entry for, is still titled by the window as today. The title rules move to src/shared (agent-row-primary-text, and the new agent-generated-tab-title-rules) so both sides share them; switch-off behaviour is unchanged. * feat(layout): main applies a phone's or paired client's tab move itself, and a tab move names the tab it follows so a stale index can't misplace it (layout-owner build, L193) * fix(layout): a move in a group showing a tab main lacks goes the window's way, a twin's agent tab keeps its window title, and orcad subscribes titles through the store attach (layout-owner build) * chore(layout): main's tab move reaches the old-client translation through its compat entry * chore(layout): mark the window-only-row move fallback as bridge code (ledger L280) * test(layout): attaching the hook store follows its rows for main's tab titles once * chore(layout): the old-client move translation is a compat entry of its own, with no cyclic re-export * fix(layout): every tab a group shows is main's, so a client's tab move never falls back to the window (layout-owner build) - Main moves a client's tab itself in a workspace it owns; a row it holds no tab for (a combined diff) keeps no place, and an order must still name each row shown once (L280). - The window publishes an editor row under its tab id only; a file no group shows gets none. - Main's layout load gives a saved browser record with no tab its tab, as the window's hydration repair did, and the window no longer mints one for a workspace main owns. - Main's layout load drops the saved tabs the window's restore drops (view-less diff, conflict and check tabs, old html previews), through one shared rule; the next save writes it back (L286). - Main lists conflict-review and check-details tabs as the plain file rows the window lists, so older clients keep them once the window's rows are gone (L287). * fix(lint): drop an import the shared restore rule left unused * test: import the move translation through the folder's public path * fix(layout): review follow-ups for the move fallback removal (layout-owner build) - A transient tab stays at load while a view is held for its file, as the window's frame build keeps it, not only for its own tab id. - The repaired browser tab goes to the saved active group only while it holds a tab, as selectHydratedActiveGroupId picks it; tests that the id is stable across loads and restarts, that a window-owned build restores it once, and that a paired server's record keeps the window's repair. - Two switch-on tests keyed editor drafts by path; drafts are keyed by editorFileKey (an owned id in a layout-owner build), so they now use it. * fix(layout): the repaired browser tab skips a saved active group this load empties * test(layout): directly dialed SSH browser record is main's; a server's SSH workspace keeps the window repair * fix(layout): drop view-less editor tabs only in loads that carry main's views, so main's own edits keep a live diff * test(layout): a direct SSH partition gets the same browser repair and drop with no host contact --- .../main-layout-session-tab-move.test.ts | 52 ++++++- .../main-layout-session-tab-move.ts | 25 ++- .../workspace-layout-projection.test.ts | 18 +++ .../runtime/workspace-layout-projection.ts | 3 +- ...ession-tab-rows-main-window-parity.test.ts | 28 +++- ...ync-runtime-graph-editor-diff-tabs.test.ts | 24 ++- .../mobile-session-snapshots.ts | 5 +- .../browser/browser-hydration-actions.ts | 5 + .../store-session-browser-hydration.test.ts | 102 ++++++++++++ .../src/store/slices/tab-group-state.ts | 8 +- .../tabs-hydration-main-load-drop.test.ts | 120 ++++++++++++++ .../src/store/slices/tabs-hydration.ts | 22 +-- .../old-client-method-translation.ts | 28 +++- .../old-client-session-editor-rows.ts | 27 ++++ ...old-client-session-tabs-projection.test.ts | 10 +- .../old-client-session-tabs-projection.ts | 9 +- .../saved-editor-tab-restore.ts | 31 ++++ ...rkspace-layout-load-browser-repair.test.ts | 147 ++++++++++++++++++ ...rkspace-layout-load-view-less-tabs.test.ts | 124 +++++++++++++++ .../workspace-layout-load-workspace.ts | 55 +++++++ .../workspace-layout-round-trip.test.ts | 4 +- 21 files changed, 781 insertions(+), 66 deletions(-) create mode 100644 src/main/runtime/workspace-layout-projection.test.ts create mode 100644 src/renderer/src/store/slices/tabs-hydration-main-load-drop.test.ts create mode 100644 src/shared/workspace-layout/saved-editor-tab-restore.ts create mode 100644 src/shared/workspace-layout/workspace-layout-load-browser-repair.test.ts create mode 100644 src/shared/workspace-layout/workspace-layout-load-view-less-tabs.test.ts diff --git a/src/main/runtime/session-tabs/main-layout-session-tab-move.test.ts b/src/main/runtime/session-tabs/main-layout-session-tab-move.test.ts index 310062dd4d7..ea3670ca836 100644 --- a/src/main/runtime/session-tabs/main-layout-session-tab-move.test.ts +++ b/src/main/runtime/session-tabs/main-layout-session-tab-move.test.ts @@ -82,37 +82,73 @@ describe("a client's tab move in a workspace main owns", () => { expect(groups().map((entry) => entry.tabOrder)).toEqual([[TAB_ID], ['a', 'b']]) }) - it('leaves a move in a group showing a tab main lacks to the window, as today', async () => { - const { runtime, moveSessionTab, groups, group } = await setup({ window: true }) + /** The client was shown a row main holds no tab for (drawn before main's frame) at the group's front. */ + function showUnheldRow(runtime: unknown): void { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the move reads only this member, which OrcaRuntimeService implements. - const rowsOf = runtime as unknown as { + const rowsOf = runtime as { toMobileSessionTabsResult: (shown: unknown) => { tabGroups?: { id: string; tabOrder: string[] }[] } } const real = rowsOf.toMobileSessionTabsResult.bind(runtime) - // The window shows a diff tab main's layout does not hold. rowsOf.toMobileSessionTabsResult = (shown) => { const rows = real(shown) return { ...rows, tabGroups: rows.tabGroups?.map((entry) => ({ ...entry, - tabOrder: [...entry.tabOrder, 'diff-1'] + tabOrder: ['diff-1', ...entry.tabOrder] })) } } - const before = groups() + } + // Why (L280): main moves it itself, never by the window's path; a row it holds no tab for keeps + // no place of its own, and the client's indices count it as the client saw it. + it('moves in a group showing a row main holds no tab for, without asking the window', async () => { + const { runtime, moveSessionTab, groups, group } = await setup({ window: true }) + showUnheldRow(runtime) + + // Shown: diff-1, terminal, a, b. Index 1 is before the terminal. await runtime.moveMobileSessionTab(WT, { kind: 'move-to-group', + tabId: 'b', + targetGroupId: group, + index: 1 + }) + expect(groups().map((entry) => entry.tabOrder)).toEqual([['b', TAB_ID, 'a']]) + + await runtime.moveMobileSessionTab(WT, { + kind: 'reorder', tabId: 'a', targetGroupId: group, - index: 0 + tabOrder: ['a', 'diff-1', 'b', TAB_ID] }) + expect(groups().map((entry) => entry.tabOrder)).toEqual([['a', 'b', TAB_ID]]) + expect(moveSessionTab).not.toHaveBeenCalled() + }) - expect(moveSessionTab).toHaveBeenCalledOnce() + it('refuses an order that does not name exactly the rows shown, moving nothing', async () => { + const { runtime, moveSessionTab, groups, group } = await setup({ window: true }) + showUnheldRow(runtime) + const before = groups() + + for (const tabOrder of [ + ['a', 'b', TAB_ID], + ['a', 'other', 'b', TAB_ID], + ['a', 'diff-1', 'diff-1', 'b', TAB_ID] + ]) { + await expect( + runtime.moveMobileSessionTab(WT, { + kind: 'reorder', + tabId: 'a', + targetGroupId: group, + tabOrder + }) + ).rejects.toThrow('invalid_tab_order') + } expect(groups()).toEqual(before) + expect(moveSessionTab).not.toHaveBeenCalled() }) it("refuses with today's codes and moves nothing", async () => { diff --git a/src/main/runtime/session-tabs/main-layout-session-tab-move.ts b/src/main/runtime/session-tabs/main-layout-session-tab-move.ts index d9228aed4c1..d3b0a0e211b 100644 --- a/src/main/runtime/session-tabs/main-layout-session-tab-move.ts +++ b/src/main/runtime/session-tabs/main-layout-session-tab-move.ts @@ -8,7 +8,6 @@ import type { RuntimeMobileSessionTabsSnapshot } from '../../../shared/runtime-types' import { translateSessionTabMove } from '../../../shared/workspace-layout/old-client/old-client-method-translation' -import { resolveSessionTabRow } from '../../../shared/workspace-layout/old-client/old-client-session-tab-rows' import { startedMainTerminalSurfaces } from '../terminal/surface/main-terminal-surfaces' import { OrcaRuntimeWithMainLayoutRenames } from './main-layout-renames' @@ -20,8 +19,8 @@ export abstract class OrcaRuntimeWithMainLayoutTabMoves extends OrcaRuntimeWithM ): ReturnType /** - * The move in main's layout, read against the rows the client was shown. Null when main does not - * own the workspace or cannot read the move: today's path moves it, or refuses it with its codes. + * The move in main's layout, checked against the rows the client was shown and refused with + * today's codes. Null for a workspace main does not own: today's path moves it. */ protected async moveTabInMainLayout( worktreeId: string, @@ -41,18 +40,14 @@ export abstract class OrcaRuntimeWithMainLayoutTabMoves extends OrcaRuntimeWithM if (!workspace) { throw new Error('tab_not_found') } - const rows = this.toMobileSessionTabsResult(shown) - // A group showing a tab main's layout lacks (a diff, a preview) moves as today, and so does a - // move main cannot read as one: today's path checks it against those rows and answers. - // Bridge (ledger L280): goes once main's layout holds every tab a group shows. - const groupsHeld = (rows.tabGroups ?? []) - .filter((group) => group.id === move.targetGroupId || group.tabOrder.includes(move.tabId)) - .every((group) => group.tabOrder.every((rowId) => resolveSessionTabRow(workspace, rowId))) - const translation = groupsHeld - ? translateSessionTabMove(worktreeId, workspace, rows, move) - : null - if (!translation?.ok) { - return null + const translation = translateSessionTabMove( + worktreeId, + workspace, + this.toMobileSessionTabsResult(shown), + move + ) + if (!translation.ok) { + throw new Error(translation.code) } for (const command of translation.commands) { if (command.type !== 'moveTab' && command.type !== 'splitGroup') { diff --git a/src/main/runtime/workspace-layout-projection.test.ts b/src/main/runtime/workspace-layout-projection.test.ts new file mode 100644 index 00000000000..9405125d39c --- /dev/null +++ b/src/main/runtime/workspace-layout-projection.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it, vi } from 'vitest' +import { runtimeLayoutLoadContext } from './workspace-layout-projection' + +vi.mock('../../shared/layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true })) + +describe('runtimeLayoutLoadContext', () => { + // L286: a partition no view was opened in since the restart still loads as views-aware, so its + // saved view-less diff tabs are dropped, not published. + it('carries views for a partition that holds none yet', () => { + const context = runtimeLayoutLoadContext( + { getPtyBindingGeneration: () => undefined, getLayoutEditorViews: () => undefined }, + 'local' + ) + + expect(context.editorViewsOf).toBeDefined() + expect(context.editorViewsOf?.('repo-1::/w')).toBeUndefined() + }) +}) diff --git a/src/main/runtime/workspace-layout-projection.ts b/src/main/runtime/workspace-layout-projection.ts index 9249e76962f..5d0894706d0 100644 --- a/src/main/runtime/workspace-layout-projection.ts +++ b/src/main/runtime/workspace-layout-projection.ts @@ -47,7 +47,8 @@ export function runtimeLayoutLoadContext( ...(generationOf ? { ptyGenerationOf: (leafId: string) => generationOf.call(store, hostId, leafId) } : {}), - ...(views ? { editorViewsOf: (key: string) => views[key] } : {}) + // A partition with no view yet still loads as one whose views are known (none held). + ...(store.getLayoutEditorViews ? { editorViewsOf: (key: string) => views?.[key] } : {}) } } diff --git a/src/renderer/src/runtime/session-tab-rows-main-window-parity.test.ts b/src/renderer/src/runtime/session-tab-rows-main-window-parity.test.ts index f559479123d..350974aae68 100644 --- a/src/renderer/src/runtime/session-tab-rows-main-window-parity.test.ts +++ b/src/renderer/src/runtime/session-tab-rows-main-window-parity.test.ts @@ -1,4 +1,4 @@ -// Diff and markdown-preview rows: main's projection of its layout model (the window's session plus +// Diff, markdown-preview, conflict-review and check-details rows: main's projection of its layout model (the window's session plus // the editor views the window publishes) must equal the rows the window builds for the same tabs. import { describe, expect, it, vi } from 'vitest' @@ -98,6 +98,25 @@ const files: OpenFile[] = [ diffSource: 'branch', isDirty: false }, + // L287: a conflict review and a check's details; older clients list both as plain file rows. + { + id: `${GIT_KEY}::conflict-review`, + filePath: '/Users/dev/orca', + relativePath: 'Merge conflicts', + worktreeId: GIT_KEY, + language: 'plaintext', + mode: 'conflict-review', + isDirty: false + }, + { + id: `${GIT_KEY}::check-details::build`, + filePath: '/Users/dev/orca', + relativePath: 'build', + worktreeId: GIT_KEY, + language: 'plaintext', + mode: 'check-details', + isDirty: false + }, { id: `${GIT_KEY}::all-diffs::uncommitted`, filePath: '/Users/dev/orca', @@ -121,7 +140,10 @@ function scenario(activeFileId: string) { tabs: files.map((file) => ({ id: tabIdOf(file), entityId: file.id, - kind: file.mode === 'diff' ? ('diff' as const) : ('editor' as const), + kind: + file.mode === 'diff' || file.mode === 'conflict-review' || file.mode === 'check-details' + ? file.mode + : ('editor' as const), entry: { color: file.mode === 'diff' ? '#00ff00' : null } })) } @@ -177,7 +199,7 @@ function published(snapshot: Pick { +describe("main's diff, markdown-preview and review rows", () => { it.each([ ['a diff', `${GIT_KEY}::diff::unstaged::src/app.ts`], ['a markdown preview', `markdown-preview::${README}`] diff --git a/src/renderer/src/runtime/sync-runtime-graph-editor-diff-tabs.test.ts b/src/renderer/src/runtime/sync-runtime-graph-editor-diff-tabs.test.ts index b606e6fdde3..904fb09a258 100644 --- a/src/renderer/src/runtime/sync-runtime-graph-editor-diff-tabs.test.ts +++ b/src/renderer/src/runtime/sync-runtime-graph-editor-diff-tabs.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../shared/layout-owner-switch' import { buildMobileSessionTabSnapshots } from './sync-runtime-graph/mobile-session-snapshots' import { makeState } from './sync-runtime-graph-test-harness' import type { AppState } from '../store/types' @@ -498,15 +499,28 @@ describe('buildMobileSessionTabSnapshots', () => { const snapshot = buildMobileSessionTabSnapshots(state)[0] + // Layout-owner build (L280): a row goes out under its tab id only, so a diff no group shows + // gets no row; the same-path edit tab keeps its own. + const diffRows = ORCA_LAYOUT_OWNER_IS_MAIN + ? [] + : [ + { + type: 'file', + id: diffId, + mode: 'diff', + diffSource: 'unstaged', + relativePath: 'src/app.ts' + } + ] expect(snapshot?.tabs).toMatchObject([ { type: 'file', id: 'editor-tab-1', mode: 'edit', relativePath: 'src/app.ts' }, - { type: 'file', id: diffId, mode: 'diff', diffSource: 'unstaged', relativePath: 'src/app.ts' } + ...diffRows ]) expect(snapshot?.tabGroups).toEqual([ { id: 'group-1', activeTabId: 'editor-tab-1', - tabOrder: ['editor-tab-1', diffId], + tabOrder: ['editor-tab-1', ...(ORCA_LAYOUT_OWNER_IS_MAIN ? [] : [diffId])], recentTabIds: ['editor-tab-1'] } ]) @@ -751,6 +765,12 @@ describe('buildMobileSessionTabSnapshots', () => { }) )[0] + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + // Layout-owner build (L280): a file no group shows as a tab gets no row. + expect(terminalSnapshot?.tabs ?? []).toEqual([]) + expect(editorSnapshot?.tabs ?? []).toEqual([]) + return + } expect(terminalSnapshot?.tabs).toMatchObject([ { type: 'file', id: fileId, relativePath: 'src/app.ts', isActive: false } ]) diff --git a/src/renderer/src/runtime/sync-runtime-graph/mobile-session-snapshots.ts b/src/renderer/src/runtime/sync-runtime-graph/mobile-session-snapshots.ts index 03da3957d22..8b27f524175 100644 --- a/src/renderer/src/runtime/sync-runtime-graph/mobile-session-snapshots.ts +++ b/src/renderer/src/runtime/sync-runtime-graph/mobile-session-snapshots.ts @@ -34,6 +34,7 @@ import { buildMobileBrowserTab } from './mobile-session-browser-tabs' import type { MobileSessionPublicationInputs } from './types' import { canReuseMobileSessionSnapshot } from './mobile-session-capture' import { selectFolderWorkspaces } from '@/store/workspace-record-selectors' +import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' export function buildMobileSessionTabSnapshots( state: AppState, @@ -172,7 +173,9 @@ export function buildMobileSessionTabSnapshots( fallbackEditorTabs.push({ tabId: fallbackTab.id, groupId: unifiedTab.groupId }) emittedEditorTabIds.add(unifiedTab.id) } - for (const file of openFilesForWorktree.values()) { + // Switch point: a row goes out under its tab id only, the id main's layout resolves (L280). + const tablessFiles = ORCA_LAYOUT_OWNER_IS_MAIN ? [] : openFilesForWorktree.values() + for (const file of tablessFiles) { if (!isMobilePublishableOpenFile(file) || emittedEditorFileIds.has(file.id)) { continue } diff --git a/src/renderer/src/store/slices/browser/browser-hydration-actions.ts b/src/renderer/src/store/slices/browser/browser-hydration-actions.ts index 73493b750bf..ff09eac78c5 100644 --- a/src/renderer/src/store/slices/browser/browser-hydration-actions.ts +++ b/src/renderer/src/store/slices/browser/browser-hydration-actions.ts @@ -10,6 +10,7 @@ import { findWorkspace } from '../browser-page-records' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../shared/layout-owner-switch' import { sendBrowserLayoutCommand } from './browser-layout-owner-commands' import { selectFolderWorkspaces } from '../../workspace-record-selectors' +import { runtimeEnvironmentIdUnlessTwin } from '@/lib/resolve-owner' export function createBrowserHydrationActions( set: BrowserSliceSet, @@ -60,6 +61,10 @@ export function createBrowserHydrationActions( // desktop was hosting pages, and the runtime only hands them back once it sees an attach. void ensureBrowserClientHostsForRestoredPages(state) for (const [worktreeId, browserTabs] of Object.entries(state.browserTabsByWorktree)) { + // Switch point: main's layout load gives a tabless record its tab; its frame draws it. + if (ORCA_LAYOUT_OWNER_IS_MAIN && !runtimeEnvironmentIdUnlessTwin(state, worktreeId)) { + continue + } for (const bt of browserTabs) { const exists = (state.unifiedTabsByWorktree[worktreeId] ?? []).some( (t) => t.contentType === 'browser' && t.entityId === bt.id diff --git a/src/renderer/src/store/slices/store-session-browser-hydration.test.ts b/src/renderer/src/store/slices/store-session-browser-hydration.test.ts index df2f42550c4..88905ed9fb2 100644 --- a/src/renderer/src/store/slices/store-session-browser-hydration.test.ts +++ b/src/renderer/src/store/slices/store-session-browser-hydration.test.ts @@ -71,6 +71,108 @@ describe('hydrateBrowserSession', () => { expect(s.activeBrowserTabId).toBe('browser-1') }) + // L280: in a layout-owner build main's layout load gives a tabless record its tab, so the window + // mints none of its own (an id main could not resolve); a window-owned build repairs it here. + // A directly dialed SSH workspace is main's too (its layout loads under its `ssh:` host). + it.each([ + ['local', {}, undefined], + [ + 'directly dialed SSH', + { connectionId: 'box', executionHostId: 'ssh:box' as const }, + 'ssh:box' as const + ] + ])( + 'leaves a tabless browser record to main in a layout-owner build, else gives it a tab (%s)', + (_name, repoHost, hostId) => { + const store = createTestStore() + const validWt = 'repo1::/path/wt1' + store.setState({ + repos: [ + { + id: 'repo1', + path: '/repo1', + displayName: 'Repo 1', + badgeColor: '#000', + addedAt: 0, + ...repoHost + } + ], + worktreesByRepo: { + repo1: [makeWorktree({ id: validWt, repoId: 'repo1', path: '/path/wt1', hostId })] + }, + activeWorktreeId: validWt + }) + + store.getState().hydrateBrowserSession({ + activeRepoId: 'repo1', + activeWorktreeId: validWt, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + browserTabsByWorktree: { + [validWt]: [ + makeBrowserTab({ id: 'browser-1', worktreeId: validWt, url: 'https://example.com' }) + ] + } + }) + + const browserTabs = (store.getState().unifiedTabsByWorktree[validWt] ?? []).filter( + (tab) => tab.contentType === 'browser' + ) + expect(browserTabs.map((tab) => tab.entityId)).toEqual( + ORCA_LAYOUT_OWNER_IS_MAIN ? [] : ['browser-1'] + ) + } + ) + + // Main repairs only the workspaces it is asked about (sendLayoutApply's owner test); a paired + // server's workspace keeps today's window repair in every build. + it.each([ + ['workspace', {}], + ['SSH workspace', { connectionId: 'vm1' }] + ])("still gives a paired server's tabless browser record a tab (%s)", (_name, sshTarget) => { + const store = createTestStore() + const serverWt = 'repo1::/srv/wt1' + store.setState({ + repos: [ + { + id: 'repo1', + path: '/srv', + displayName: 'Server repo', + badgeColor: '#000', + addedAt: 0, + executionHostId: 'runtime:env-1', + ...sshTarget + } + ], + worktreesByRepo: { + repo1: [ + makeWorktree({ id: serverWt, repoId: 'repo1', path: '/srv/wt1', hostId: 'runtime:env-1' }) + ] + }, + activeWorktreeId: serverWt + }) + + store.getState().hydrateBrowserSession({ + activeRepoId: 'repo1', + activeWorktreeId: serverWt, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + browserTabsByWorktree: { + [serverWt]: [ + makeBrowserTab({ id: 'browser-1', worktreeId: serverWt, url: 'https://example.com' }) + ] + } + }) + + expect( + (store.getState().unifiedTabsByWorktree[serverWt] ?? []) + .filter((tab) => tab.contentType === 'browser') + .map((tab) => tab.entityId) + ).toEqual(['browser-1']) + }) + it('synthesizes a page for a browser workspace whose persisted page list is empty', () => { // Why: session salvage drops a corrupt page by rebuilding the array, so the // key survives holding []. Treating that as "has pages" restores a workspace diff --git a/src/renderer/src/store/slices/tab-group-state.ts b/src/renderer/src/store/slices/tab-group-state.ts index 487ad589776..083a7b4a62c 100644 --- a/src/renderer/src/store/slices/tab-group-state.ts +++ b/src/renderer/src/store/slices/tab-group-state.ts @@ -1,4 +1,4 @@ -import type { Tab, TabContentType, TabGroup } from '../../../../shared/tab-types' +import type { Tab, TabGroup } from '../../../../shared/tab-types' import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' import { createBrowserUuid } from '@/lib/browser-uuid' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' @@ -163,11 +163,7 @@ export function updateGroup(groups: TabGroup[], updated: TabGroup): TabGroup[] { return groups.map((g) => (g.id === updated.id ? updated : g)) } -export function isTransientEditorContentType(contentType: TabContentType): boolean { - return ( - contentType === 'diff' || contentType === 'conflict-review' || contentType === 'check-details' - ) -} +export { isTransientEditorContentType } from '../../../../shared/workspace-layout/saved-editor-tab-restore' export function getPersistedEditFileIdsByWorktree( session: WorkspaceSessionState diff --git a/src/renderer/src/store/slices/tabs-hydration-main-load-drop.test.ts b/src/renderer/src/store/slices/tabs-hydration-main-load-drop.test.ts new file mode 100644 index 00000000000..f40b00dbccf --- /dev/null +++ b/src/renderer/src/store/slices/tabs-hydration-main-load-drop.test.ts @@ -0,0 +1,120 @@ +// L286: a layout-owner build's main drops view-less saved editor tabs at load and writes that back. +// A window-owned build reading the written session must restore exactly what it did before. + +import { describe, expect, it, vi } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../../shared/constants' +import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host' +import type { Tab, TabContentType } from '../../../../shared/tab-types' +import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' +import { loadWorkspaceLayout } from '../../../../shared/workspace-layout/workspace-layout-load' +import { nameBasedLoadContext } from '../../../../shared/workspace-layout/workspace-layout-minted-ids' +import { saveWorkspaceLayout } from '../../../../shared/workspace-layout/workspace-layout-save' +import { buildHydratedTabState } from './tabs-hydration' + +const layoutOwner = vi.hoisted(() => ({ isMain: true })) +vi.mock('../../../../shared/layout-owner-switch', () => ({ + get ORCA_LAYOUT_OWNER_IS_MAIN() { + return layoutOwner.isMain + } +})) + +const KEY = 'repo-1::/w' + +function entry(id: string, contentType: TabContentType, entityId: string, sortOrder: number): Tab { + return { + id, + entityId, + groupId: 'g1', + worktreeId: KEY, + contentType, + label: id, + customLabel: null, + color: null, + sortOrder, + createdAt: 1 + } +} + +function savedSession(): WorkspaceSessionState { + const tabs = [ + entry('ghost-diff', 'diff', '/w/gone.ts', 0), + entry('ed-a', 'editor', '/w/a.ts', 1), + entry('ghost-review', 'conflict-review', '/w/conflict', 2), + entry('kept-diff', 'diff', '/w/b.ts', 3), + entry('ed-b', 'editor', '/w/b.ts', 4) + ] + return { + ...getDefaultWorkspaceSession(), + unifiedTabs: { [KEY]: tabs }, + tabGroups: { + [KEY]: [{ id: 'g1', worktreeId: KEY, activeTabId: 'ed-a', tabOrder: tabs.map((t) => t.id) }] + }, + openFilesByWorktree: { + [KEY]: ['/w/a.ts', '/w/b.ts'].map((filePath) => ({ + filePath, + relativePath: filePath.slice(3), + worktreeId: KEY, + language: 'typescript' + })) + } + } +} + +describe("a window-owned build reading a session main's load rewrote", () => { + it('restores the same tabs and groups as from the session before the rewrite', () => { + layoutOwner.isMain = true + const rewritten = saveWorkspaceLayout( + // As main's command applies load it after a restart: its views carried, none held. + loadWorkspaceLayout(LOCAL_EXECUTION_HOST_ID, savedSession(), { + ...nameBasedLoadContext(), + editorViewsOf: () => undefined + }) + ) + expect(rewritten.unifiedTabs?.[KEY]).toHaveLength(3) + layoutOwner.isMain = false + + const restore = (session: WorkspaceSessionState) => { + const state = buildHydratedTabState(session, new Set([KEY])) + return { + tabs: state.unifiedTabsByWorktree[KEY]?.map((tab) => [tab.id, tab.contentType]), + groups: state.groupsByWorktree[KEY]?.map((group) => group.tabOrder) + } + } + expect(restore(rewritten)).toEqual(restore(savedSession())) + expect(restore(rewritten).groups).toEqual([['ed-a', 'kept-diff', 'ed-b']]) + }) + + // L280: main's load also gives a tabless browser record its tab and saves it; a window-owned + // build then restores that one tab and has nothing left to repair. + it("restores the browser tab main's load added, once", () => { + layoutOwner.isMain = true + const withRecord: WorkspaceSessionState = { + ...savedSession(), + browserTabsByWorktree: { + [KEY]: [ + { + id: 'browser-1', + worktreeId: KEY, + url: 'https://example.com', + title: 'Example', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 5 + } + ] + } + } + const loaded = loadWorkspaceLayout(LOCAL_EXECUTION_HOST_ID, withRecord, nameBasedLoadContext()) + const mainId = loaded.layout.workspaces[KEY]!.tabs.find((tab) => tab.kind === 'browser')!.id + const rewritten = saveWorkspaceLayout(loaded) + layoutOwner.isMain = false + + const browserTabs = buildHydratedTabState(rewritten, new Set([KEY])).unifiedTabsByWorktree[ + KEY + ]!.filter((tab) => tab.contentType === 'browser') + expect(browserTabs.map((tab) => [tab.id, tab.entityId])).toEqual([[mainId, 'browser-1']]) + }) +}) diff --git a/src/renderer/src/store/slices/tabs-hydration.ts b/src/renderer/src/store/slices/tabs-hydration.ts index b4999e6870b..e8b21070a7b 100644 --- a/src/renderer/src/store/slices/tabs-hydration.ts +++ b/src/renderer/src/store/slices/tabs-hydration.ts @@ -11,11 +11,11 @@ import { import { dedupeEditorTabsWithinGroups, getPersistedEditFileIdsByWorktree, - isTransientEditorContentType, sanitizeRecentTabIds, selectHydratedActiveGroupId } from './tab-group-state' import { dedupeTabOrder } from '../../../../shared/workspace-layout/tab-order' +import { restoresSavedContentTab } from '../../../../shared/workspace-layout/saved-editor-tab-restore' type HydratedTabState = { unifiedTabsByWorktree: Record @@ -127,22 +127,10 @@ export function buildHydratedTabState( // containing "::"; those are invalid pane-key tab ids. return isValidTerminalTabId(tab.id) && isValidTerminalTabId(tab.entityId) } - // Why dropped rather than converted: a preview used to be an editor tab whose id encoded - // the document, and its document was never persisted — so this chrome has always come back - // naming a file no restore produces, which is the empty pane the surface was reported for. - // A preview is a browser tab now, and the worktree id inside that encoded id can itself - // contain the separator, so re-deriving the document from it is guesswork. The reader - // reopens the preview; nothing is left pointing at a surface that cannot exist. - if (tab.contentType === 'editor' && tab.entityId.startsWith('html-preview::')) { - return false - } - if (!isTransientEditorContentType(tab.contentType)) { - return true - } - // Why: restore skips backing editor state for transient diff/conflict - // items. Hydration must drop their tab chrome too or the split group - // comes back pointing at a document that no longer exists. - return persistedEditFileIds.has(tab.entityId) || keepsTransientTab(worktreeId, tab.entityId) + // The same rule main's layout load applies, so neither holds a tab the other drops. + return restoresSavedContentTab(tab, persistedEditFileIds, () => + keepsTransientTab(worktreeId, tab.entityId) + ) }) .sort((a, b) => a.sortOrder - b.sortOrder || a.createdAt - b.createdAt) // Why after the sort: the surviving record is the one the strip renders first. diff --git a/src/shared/workspace-layout/old-client/old-client-method-translation.ts b/src/shared/workspace-layout/old-client/old-client-method-translation.ts index 65924b6218b..3199fb3a93b 100644 --- a/src/shared/workspace-layout/old-client/old-client-method-translation.ts +++ b/src/shared/workspace-layout/old-client/old-client-method-translation.ts @@ -195,12 +195,12 @@ export function translateSessionTabMove( } const shownOrder = toTabIds(workspace, shown.tabOrder) if (params.kind === 'move-to-group') { - const shownIndex = Math.max(0, Math.min(params.index ?? shownOrder.length, shownOrder.length)) - // A shown index lands before the same tab in the full order; the end stays the end. - const index = - shownIndex === shownOrder.length - ? target.tabOrder.length - : target.tabOrder.indexOf(shownOrder[shownIndex]!) + const rowCount = shown.tabOrder.length + const shownIndex = Math.max(0, Math.min(params.index ?? rowCount, rowCount)) + // A shown index lands before the first tab main holds from that row on (a row it holds no tab + // for, such as one the window drew before main's frame, places nothing); past the last, the end. + const before = toTabIds(workspace, shown.tabOrder.slice(shownIndex))[0] + const index = before === undefined ? target.tabOrder.length : target.tabOrder.indexOf(before) return to({ ...on, type: 'moveTab', @@ -209,11 +209,23 @@ export function translateSessionTabMove( afterTabId: tabOrderAnchor(target.tabOrder, index) }) } - const sent = params.tabOrder.map((rowId) => resolveSessionTabRow(workspace, rowId)?.tab.id) + // A row main holds no tab for (one drawn before main's frame) is named once, and keeps no place. + const unheld = (rowIds: readonly string[]) => + rowIds.filter((rowId) => !resolveSessionTabRow(workspace, rowId)) + const shownUnheld = new Set(unheld(shown.tabOrder)) + const sentUnheld = unheld(params.tabOrder) + if ( + sentUnheld.length !== shownUnheld.size || + new Set(sentUnheld).size !== sentUnheld.length || + sentUnheld.some((rowId) => !shownUnheld.has(rowId)) + ) { + return refused('invalid_tab_order') + } + const sent = toTabIds(workspace, params.tabOrder) if ( sent.length !== shownOrder.length || new Set(sent).size !== sent.length || - sent.some((id) => id === undefined || !shownOrder.includes(id)) || + sent.some((id) => !shownOrder.includes(id)) || !sent.includes(tab.id) ) { return refused('invalid_tab_order') diff --git a/src/shared/workspace-layout/old-client/old-client-session-editor-rows.ts b/src/shared/workspace-layout/old-client/old-client-session-editor-rows.ts index 7075e4d3013..7a6ee8829a2 100644 --- a/src/shared/workspace-layout/old-client/old-client-session-editor-rows.ts +++ b/src/shared/workspace-layout/old-client/old-client-session-editor-rows.ts @@ -140,3 +140,30 @@ export function diffTabRow( ...tabRowProps(tab, scope) } } + +/** + * A conflict-review or check-details tab's row from its view: a plain file row, as the window lists + * it (buildMobileFileTab), so an older client keeps seeing the tab once the window's rows are gone. + */ +export function reviewTabRow( + tab: LayoutContentTab & { kind: 'conflict-review' | 'check-details' }, + scope: EditorRowScope +): RuntimeMobileSessionFileTab | null { + const editorView = scope.workspace.editorViews?.[tab.id] + if (editorView?.view.mode !== tab.kind) { + return null + } + const { file } = editorView + return { + type: 'file', + id: tab.id, + title: fileName(file.relativePath, 'File'), + filePath: file.filePath, + relativePath: file.relativePath, + language: file.language, + mode: 'edit', + // The model holds no unsaved edit of a review; the window's row it lists wins. + isDirty: false, + ...tabRowProps(tab, scope) + } +} diff --git a/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.test.ts b/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.test.ts index dca0ef3221f..2857c5656ab 100644 --- a/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.test.ts +++ b/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.test.ts @@ -7,6 +7,7 @@ import { type SessionTabsView } from './old-client-session-tabs-projection' import { BLANK_BROWSER_TAB_STATE } from '../workspace-layout-beside' +import { editorFileKey } from '../workspace-layout-editor-tab-file' import { build, testContext, WS } from '../workspace-layout-command.test-fixture' import type { WorkspaceLayoutModel } from '../workspace-layout-model' @@ -115,7 +116,14 @@ describe('projectSessionTabs', () => { } }, panes: { [apiTab]: { activeLeafId: apiSplit, expandedLeafId: null } }, - editorDrafts: { [WS]: { '/Users/dev/orca/README.md': { dirtyDraftContent: '# draft' } } } + // Keyed as editorFileKey keys it (an owned id in a layout-owner build), as the Loader does. + editorDrafts: { + [WS]: { + [editorFileKey({ filePath: '/Users/dev/orca/README.md' })]: { + dirtyDraftContent: '# draft' + } + } + } }) const facts: SessionTabsFacts = { tabLabels: {}, diff --git a/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.ts b/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.ts index 1c20f12625f..eff1eb4b25d 100644 --- a/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.ts +++ b/src/shared/workspace-layout/old-client/old-client-session-tabs-projection.ts @@ -18,7 +18,7 @@ import type { } from '../../runtime-session-contracts' import type { TerminalLayoutSnapshot } from '../../terminal-tab-types' import { pruneGroupLayout } from '../../workspace-session-terminal-tab-close' -import { diffTabRow, editorTabRow } from './old-client-session-editor-rows' +import { diffTabRow, editorTabRow, reviewTabRow } from './old-client-session-editor-rows' import { agentSessionRowId, publishedTabOrderId, terminalRowId } from './old-client-session-row-ids' import { collectLayoutLeafIdsInOrder } from '../terminal-pane-tree' import { groupsInLayoutOrder } from './old-client-group-order' @@ -157,9 +157,12 @@ function projectTab(tab: LayoutTab, scope: Scope): RuntimeMobileSessionSnapshotT const row = agentSessionRow(tab, scope) return row ? [row] : [] } - // These need the desktop to draw them. case 'conflict-review': - case 'check-details': + case 'check-details': { + const row = reviewTabRow({ ...tab, kind: tab.kind }, scope) + return row ? [row] : [] + } + // A simulator needs the desktop to draw it; the window lists none either. case 'simulator': return [] } diff --git a/src/shared/workspace-layout/saved-editor-tab-restore.ts b/src/shared/workspace-layout/saved-editor-tab-restore.ts new file mode 100644 index 00000000000..4b53d9b217b --- /dev/null +++ b/src/shared/workspace-layout/saved-editor-tab-restore.ts @@ -0,0 +1,31 @@ +// Which saved editor-family tabs a restore brings back. One rule for the window's hydration and +// main's layout load, so both drop the same tabs and neither holds a tab the other cannot draw. + +import type { Tab, TabContentType } from '../tab-types' + +/** Diff, conflict and check tabs: their documents live in memory only and are never saved. */ +export function isTransientEditorContentType(contentType: TabContentType): boolean { + return ( + contentType === 'diff' || contentType === 'conflict-review' || contentType === 'check-details' + ) +} + +/** + * Whether a saved non-terminal tab comes back. An old html preview's tab names a document no restore + * produces (a preview is a browser tab now, and re-deriving its document from the id is guesswork). + * A transient tab's document is never saved, so it comes back only for a saved edit file of its + * entity, or while its view is still held (`keepsView`); else its group would point at nothing. + */ +export function restoresSavedContentTab( + tab: Pick, + savedEditFilePaths: ReadonlySet, + keepsView: () => boolean +): boolean { + if (tab.contentType === 'editor' && tab.entityId.startsWith('html-preview::')) { + return false + } + if (!isTransientEditorContentType(tab.contentType)) { + return true + } + return savedEditFilePaths.has(tab.entityId) || keepsView() +} diff --git a/src/shared/workspace-layout/workspace-layout-load-browser-repair.test.ts b/src/shared/workspace-layout/workspace-layout-load-browser-repair.test.ts new file mode 100644 index 00000000000..9af7b5b2f55 --- /dev/null +++ b/src/shared/workspace-layout/workspace-layout-load-browser-repair.test.ts @@ -0,0 +1,147 @@ +// Layout-owner build (L280): a saved browser record with no tab-bar entry gets its tab from main's +// load, as the window's browser hydration repairs it, so no window mints an id main cannot resolve. + +import { describe, expect, it, vi } from 'vitest' +import { getDefaultWorkspaceSession } from '../constants' +import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../execution-host' +import type { Tab } from '../tab-types' +import type { WorkspaceSessionState } from '../workspace-session-state-types' +import { resolveSessionTabRow } from './old-client/old-client-session-tab-rows' +import { loadWorkspaceLayout } from './workspace-layout-load' +import { nameBasedLoadContext } from './workspace-layout-minted-ids' +import { saveWorkspaceLayout } from './workspace-layout-save' + +vi.mock('../layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true })) + +const KEY = 'repo-1::/w' + +function editor(id: string, groupId: string, sortOrder: number): Tab { + return { + id, + entityId: `/w/${id}.ts`, + groupId, + worktreeId: KEY, + contentType: 'editor', + label: id, + customLabel: null, + color: null, + sortOrder, + createdAt: 1 + } +} + +/** Two groups, the second active, and a browser record no tab-bar entry names. */ +function savedSession(): WorkspaceSessionState { + return { + ...getDefaultWorkspaceSession(), + unifiedTabs: { [KEY]: [editor('a', 'g1', 0), editor('b', 'g2', 0)] }, + tabGroups: { + [KEY]: [ + { id: 'g1', worktreeId: KEY, activeTabId: 'a', tabOrder: ['a'] }, + { id: 'g2', worktreeId: KEY, activeTabId: 'b', tabOrder: ['b'] } + ] + }, + activeGroupIdByWorktree: { [KEY]: 'g2' }, + openFilesByWorktree: { + [KEY]: ['a', 'b'].map((name) => ({ + filePath: `/w/${name}.ts`, + relativePath: `${name}.ts`, + worktreeId: KEY, + language: 'typescript' + })) + }, + browserTabsByWorktree: { + [KEY]: [ + { + id: 'browser-1', + worktreeId: KEY, + url: 'https://example.com', + title: 'Example', + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 5 + } + ] + } + } +} + +// As main's projection loads it after a restart: its views carried, none held. +const load = (session: WorkspaceSessionState, hostId: ExecutionHostId = LOCAL_EXECUTION_HOST_ID) => + loadWorkspaceLayout(hostId, session, { + ...nameBasedLoadContext(), + editorViewsOf: () => undefined + }) + +describe("main's load of a browser record with no tab", () => { + it('gives it a tab at the end of the active group, which a row id resolves to', () => { + const workspace = load(savedSession()).layout.workspaces[KEY]! + const tab = workspace.tabs.find((entry) => entry.kind === 'browser')! + + expect(tab).toMatchObject({ entityId: 'browser-1', createdAt: 5 }) + expect(workspace.groups.map((group) => group.tabOrder)).toEqual([['a'], ['b', tab.id]]) + // The window publishes a browser row under its record id. + expect(resolveSessionTabRow(workspace, 'browser-1')?.tab.id).toBe(tab.id) + }) + + it('saves the tab, so the next load mints nothing new', () => { + const saved = saveWorkspaceLayout(load(savedSession())) + + expect( + saved.unifiedTabs?.[KEY]?.find((entry) => entry.contentType === 'browser') + ).toMatchObject({ entityId: 'browser-1', groupId: 'g2', label: 'Example' }) + expect(load(saved).changes).toEqual([]) + }) + + it('mints the same id on every load, and a restart after a save keeps one tab', () => { + const ids = (session: WorkspaceSessionState) => + load(session) + .layout.workspaces[KEY]!.tabs.filter((tab) => tab.kind === 'browser') + .map((tab) => tab.id) + const first = ids(savedSession()) + + expect(ids(savedSession())).toEqual(first) + expect(ids(saveWorkspaceLayout(load(savedSession())))).toEqual(first) + expect(first).toHaveLength(1) + }) + + // The load reads only the saved partition: no host contact, so a direct SSH host that is down + // (unverifiable, not gone) gets the same repair and loses nothing but the window's drop. + it('repairs and drops the same in a direct SSH partition, whose host it never contacts', () => { + const session = savedSession() + session.unifiedTabs![KEY]!.push({ + ...editor('ghost', 'g2', 1), + contentType: 'diff', + entityId: '/w/gone.ts' + }) + const tabsOf = (hostId: ExecutionHostId) => + load(session, hostId).layout.workspaces[KEY]!.tabs.map((tab) => [tab.id, tab.kind]) + + expect(tabsOf('ssh:box')).toEqual(tabsOf(LOCAL_EXECUTION_HOST_ID)) + expect(tabsOf('ssh:box')).toContainEqual([expect.any(String), 'browser']) + expect(tabsOf('ssh:box').map(([id]) => id)).toEqual(expect.arrayContaining(['a', 'b'])) + expect(tabsOf('ssh:box').map(([id]) => id)).not.toContain('ghost') + }) + + it('skips a saved active group this load empties, as the window picks a group that holds a tab', () => { + const session = savedSession() + // g1 holds only a diff tab whose document was never saved, so the load drops it. + session.unifiedTabs![KEY] = [ + { ...editor('a', 'g1', 0), id: 'ghost', contentType: 'diff', entityId: '/w/gone.ts' }, + editor('b', 'g2', 0) + ] + session.tabGroups![KEY] = [ + { id: 'g1', worktreeId: KEY, activeTabId: 'ghost', tabOrder: ['ghost'] }, + { id: 'g2', worktreeId: KEY, activeTabId: 'b', tabOrder: ['b'] } + ] + session.activeGroupIdByWorktree = { [KEY]: 'g1' } + + const workspace = load(session).layout.workspaces[KEY]! + const tab = workspace.tabs.find((entry) => entry.kind === 'browser')! + + expect(workspace.groups).toEqual([{ id: 'g2', tabOrder: ['b', tab.id] }]) + }) +}) diff --git a/src/shared/workspace-layout/workspace-layout-load-view-less-tabs.test.ts b/src/shared/workspace-layout/workspace-layout-load-view-less-tabs.test.ts new file mode 100644 index 00000000000..5a4a3023a4c --- /dev/null +++ b/src/shared/workspace-layout/workspace-layout-load-view-less-tabs.test.ts @@ -0,0 +1,124 @@ +// Layout-owner build (L286): main's layout load, given its views, drops the saved tabs the window's +// restore drops (restoresSavedContentTab), so main never holds a tab no window draws. + +import { describe, expect, it, vi } from 'vitest' +import { LOCAL_EXECUTION_HOST_ID } from '../execution-host' +import type { Tab, TabContentType } from '../tab-types' +import type { WorkspaceSessionState } from '../workspace-session-state-types' +import { getDefaultWorkspaceSession } from '../constants' +import { translateSessionTabMove } from './old-client/old-client-method-translation' +import { loadWorkspaceLayout } from './workspace-layout-load' +import { nameBasedLoadContext } from './workspace-layout-minted-ids' +import { saveWorkspaceLayout } from './workspace-layout-save' + +vi.mock('../layout-owner-switch', () => ({ ORCA_LAYOUT_OWNER_IS_MAIN: true })) + +const KEY = 'repo-1::/w' + +function entry(id: string, contentType: TabContentType, entityId: string, sortOrder: number): Tab { + return { + id, + entityId, + groupId: 'g1', + worktreeId: KEY, + contentType, + label: id, + customLabel: null, + color: null, + sortOrder, + createdAt: 1 + } +} + +/** A saved session after a restart: two edit files, and tabs whose documents were never saved. */ +function savedSession(): WorkspaceSessionState { + const tabs = [ + entry('ghost-diff', 'diff', '/w/gone.ts', 0), + entry('ed-a', 'editor', '/w/a.ts', 1), + entry('ghost-review', 'conflict-review', '/w/conflict', 2), + entry('ghost-checks', 'check-details', '/w/checks', 3), + entry('ghost-html', 'editor', 'html-preview::/w/page.html', 4), + entry('kept-diff', 'diff', '/w/b.ts', 5), + entry('ed-b', 'editor', '/w/b.ts', 6) + ] + return { + ...getDefaultWorkspaceSession(), + unifiedTabs: { [KEY]: tabs }, + tabGroups: { + [KEY]: [{ id: 'g1', worktreeId: KEY, activeTabId: 'ed-a', tabOrder: tabs.map((t) => t.id) }] + }, + openFilesByWorktree: { + [KEY]: ['/w/a.ts', '/w/b.ts'].map((filePath) => ({ + filePath, + relativePath: filePath.slice(3), + worktreeId: KEY, + language: 'typescript' + })) + } + } +} + +// Main's projection and command applies carry its views; after a restart it holds none. +const load = (session: WorkspaceSessionState) => + loadWorkspaceLayout(LOCAL_EXECUTION_HOST_ID, session, { + ...nameBasedLoadContext(), + editorViewsOf: () => undefined + }) + +describe("main's layout load of view-less saved editor tabs", () => { + it('drops the tabs the window drops and keeps a diff of a saved edit file', () => { + const workspace = load(savedSession()).layout.workspaces[KEY]! + + expect(workspace.groups.map((group) => group.tabOrder)).toEqual([['ed-a', 'kept-diff', 'ed-b']]) + expect(workspace.tabs.map((tab) => tab.id).sort()).toEqual(['ed-a', 'ed-b', 'kept-diff']) + }) + + it('writes the drop back on its next save, and a reload does not bring them back', () => { + const saved = saveWorkspaceLayout(load(savedSession())) + + expect(saved.unifiedTabs?.[KEY]?.map((tab) => tab.id)).toEqual(['ed-a', 'kept-diff', 'ed-b']) + expect(saved.tabGroups?.[KEY]?.[0]?.tabOrder).toEqual(['ed-a', 'kept-diff', 'ed-b']) + expect(load(saved).changes).toEqual([]) + }) + + // Main's own edits (a phone's browser tab, an agent tab) load without its views: they must not + // drop a live diff whose view only main's memory holds. + it('keeps every saved tab in a load that carries no views', () => { + const workspace = loadWorkspaceLayout( + LOCAL_EXECUTION_HOST_ID, + savedSession(), + nameBasedLoadContext() + ).layout.workspaces[KEY]! + + expect(workspace.tabs).toHaveLength(7) + }) + + it('keeps a transient tab while main still holds its view', () => { + const workspace = loadWorkspaceLayout(LOCAL_EXECUTION_HOST_ID, savedSession(), { + ...nameBasedLoadContext(), + editorViewsOf: () => ({ + 'ghost-diff': { + view: { mode: 'diff' }, + file: { filePath: '/w/gone.ts', relativePath: 'gone.ts', language: 'typescript' } + } + }) + }).layout.workspaces[KEY]! + + expect(workspace.groups[0]?.tabOrder).toEqual(['ghost-diff', 'ed-a', 'kept-diff', 'ed-b']) + }) + + it("does not count a view-less tab in a client's move index", () => { + const workspace = load(savedSession()).layout.workspaces[KEY]! + // The rows a client was shown: the window draws no view-less tab either. + const shown = { id: 'g1', activeTabId: 'ed-a', tabOrder: ['ed-a', 'kept-diff', 'ed-b'] } + + const translation = translateSessionTabMove( + KEY, + workspace, + { tabGroups: [shown] }, + { kind: 'move-to-group', tabId: 'ed-b', targetGroupId: 'g1', index: 0 } + ) + + expect(translation).toMatchObject({ ok: true, commands: [{ type: 'moveTab', index: 0 }] }) + }) +}) diff --git a/src/shared/workspace-layout/workspace-layout-load-workspace.ts b/src/shared/workspace-layout/workspace-layout-load-workspace.ts index fb22a919b03..379ea60b4a0 100644 --- a/src/shared/workspace-layout/workspace-layout-load-workspace.ts +++ b/src/shared/workspace-layout/workspace-layout-load-workspace.ts @@ -7,6 +7,7 @@ import type { WorkspaceSessionState } from '../workspace-session-state-types' import { childRecord, pickStoredFields } from './stored-record-fields' import { readCanonicalPtyOwners, resolveCanonicalPtyClaim } from './legacy-terminal-row-claims' import { resolveGroupOrder, type OrderCandidate } from './workspace-layout-load-order' +import { resolveTabGroupOwners } from './tab-group-owners' import { loadBrowserTabs, loadEditorFiles, @@ -18,6 +19,7 @@ import { loadTerminalTab } from './workspace-layout-load-tabs' import type { WorkspaceLoadArgs } from './workspace-layout-load-types' +import { restoresSavedContentTab } from './saved-editor-tab-restore' import type { LayoutTab, LayoutTerminalTab, WorkspaceLayout } from './workspace-layout-model' import { pruneGroupLayout } from '../workspace-session-terminal-tab-close' @@ -100,6 +102,18 @@ function loadTabs(args: WorkspaceLoadArgs): { const rows = takeRows(args) const rowByEntity = new Map(rows.map((row) => [row.id, row])) const merged = new Set() + const savedEditFilePaths = new Set( + (session.openFilesByWorktree?.[key] ?? []).map((file) => file.filePath) + ) + // A transient tab stays while a view is held for its file, as the window's frame build keeps it. + // Only a load that carries main's views drops one: a load without them keeps every stored tab. + const viewsOf = ORCA_LAYOUT_OWNER_IS_MAIN ? args.context.editorViewsOf : undefined + const heldViews = viewsOf?.(key) ?? {} + const viewedEntities = new Set( + (session.unifiedTabs?.[key] ?? []).flatMap((entry) => + heldViews[entry.id] ? [entry.entityId] : [] + ) + ) const tabs: LayoutTab[] = [] const candidates: OrderCandidate[] = [] const tabIds = new Set() @@ -128,6 +142,14 @@ function loadTabs(args: WorkspaceLoadArgs): { if (tabIds.has(entry.id) || aliases.has(entry.id)) { continue } + // Switch point: main drops the saved tabs the window's restore drops (L286). + if ( + viewsOf && + entry.contentType !== 'terminal' && + !restoresSavedContentTab(entry, savedEditFilePaths, () => viewedEntities.has(entry.entityId)) + ) { + continue + } if (entry.contentType === 'terminal') { const row = rowByEntity.get(entry.entityId) if (!row || merged.has(row.id)) { @@ -151,6 +173,39 @@ function loadTabs(args: WorkspaceLoadArgs): { childRecord(args.view.lastFocusedAt, key)[entry.id] = entry.lastFocusedAt } } + // Switch point: a saved browser record with no tab gets one at the end of the active group (the + // saved one while it holds a tab, as selectHydratedActiveGroupId picks it), the repair the + // window's browser hydration makes (createUnifiedTab), so no window mints its own id. + if (ORCA_LAYOUT_OWNER_IS_MAIN) { + const storedGroups = session.tabGroups?.[key] ?? [] + const heldGroupIds = new Set(resolveTabGroupOwners(candidates, storedGroups).values()) + const savedActive = session.activeGroupIdByWorktree?.[key] + const activeGroupId = + savedActive && heldGroupIds.has(savedActive) + ? savedActive + : storedGroups.find((group) => heldGroupIds.has(group.id))?.id + for (const record of session.browserTabsByWorktree?.[key] ?? []) { + if (tabs.some((tab) => tab.kind === 'browser' && tab.entityId === record.id)) { + continue + } + const id = args.context.mintId(`browser-tab:${record.id}`) + tabs.push({ + id, + kind: 'browser', + entityId: record.id, + createdAt: record.createdAt, + customTitle: null, + color: null + }) + tabIds.add(id) + childRecord(args.facts.tabLabels, key)[id] = record.title + candidates.push({ + id, + groupId: activeGroupId, + createdAt: record.createdAt + }) + } + } // A row outside the tab bar whose every terminal a tab-bar tab holds is a stale copy of it. const canonical = readCanonicalPtyOwners(session, key, rows) for (const row of rows) { diff --git a/src/shared/workspace-layout/workspace-layout-round-trip.test.ts b/src/shared/workspace-layout/workspace-layout-round-trip.test.ts index daab37e912a..334db2b87a9 100644 --- a/src/shared/workspace-layout/workspace-layout-round-trip.test.ts +++ b/src/shared/workspace-layout/workspace-layout-round-trip.test.ts @@ -5,6 +5,7 @@ import { loadWorkspaceLayout } from './workspace-layout-load' import { checkWorkspaceLayoutModelRules } from './workspace-layout-model-rules' import { checkWorkspaceLayoutRules } from './workspace-layout-rules' import { saveWorkspaceLayout } from './workspace-layout-save' +import { editorFileKey } from './workspace-layout-editor-tab-file' import { localDesktopSession, WORKSPACE_LAYOUT_PROFILES @@ -105,8 +106,9 @@ describe('workspace layout Loader and Serializer', () => { activeTabId: 'tab-agent', recentTabIds: ['tab-shell', 'tab-agent'] }) + // Keyed as editorFileKey keys it: a layout-owner build keys by owned file id. expect(loaded.desktopView.editorDrafts['repo-1::/Users/dev/orca']).toEqual({ - '/Users/dev/orca/src/app.ts': { + [editorFileKey({ filePath: '/Users/dev/orca/src/app.ts' })]: { dirtyDraftContent: 'export {}\n', lastKnownDiskSignature: 'sig-1' }