diff --git a/apps/desktop/src/browser-guests.ts b/apps/desktop/src/browser-guests.ts index d3824dd902..027b7cee5b 100644 --- a/apps/desktop/src/browser-guests.ts +++ b/apps/desktop/src/browser-guests.ts @@ -54,7 +54,7 @@ export class DesktopBrowserGuests { this.leases.delete(key) const guest = lease.guest if (guest !== undefined && !guest.isDestroyed()) { - const destroyed = new Promise(resolve => { guest.once('destroyed', resolve) }) + const destroyed = new Promise((resolve) => { guest.once('destroyed', resolve) }) guest.close({ waitForBeforeUnload: false }) await destroyed } diff --git a/apps/desktop/tests/preload-app.spec.ts b/apps/desktop/tests/preload-app.spec.ts index 6e1740fc61..f1652d89ea 100644 --- a/apps/desktop/tests/preload-app.spec.ts +++ b/apps/desktop/tests/preload-app.spec.ts @@ -27,7 +27,8 @@ it('limits product documents to update status and a native confirmation action', expect(api.updates).not.toHaveProperty('install') const listener = vi.fn() const dispose = api.updates.subscribe(listener) - const handler = electron.ipcRenderer.on.mock.calls.find(([channel]) => channel === DESKTOP_IPC.updatesPresentation)?.[1] as (event: unknown, state: unknown) => void + const handler = electron.ipcRenderer.on.mock.calls.find(([channel]) => channel === DESKTOP_IPC.updatesPresentation)?.[1] as + (event: unknown, state: unknown) => void handler({}, { visible: false }) expect(listener).toHaveBeenCalledWith({ visible: false }) dispose() diff --git a/packages/client/ui-dockkit/src/components/FloatLayer.tsx b/packages/client/ui-dockkit/src/components/FloatLayer.tsx index 4253880f0b..5f512423bd 100644 --- a/packages/client/ui-dockkit/src/components/FloatLayer.tsx +++ b/packages/client/ui-dockkit/src/components/FloatLayer.tsx @@ -124,7 +124,7 @@ export function FloatHeader({ paneId, tab, labels, intents, renderTabTitle, canC
@@ -132,7 +132,7 @@ export function FloatHeader({ paneId, tab, labels, intents, renderTabTitle, canC {(canCloseTab?.(tab.id) ?? true) && ( diff --git a/packages/client/ui-dockkit/src/components/TabLayout.tsx b/packages/client/ui-dockkit/src/components/TabLayout.tsx index c88e206f04..36f37a4ab2 100644 --- a/packages/client/ui-dockkit/src/components/TabLayout.tsx +++ b/packages/client/ui-dockkit/src/components/TabLayout.tsx @@ -35,7 +35,8 @@ interface TabHostProps extends LayoutProps { } /** A tab's ancestors stay identical across selection, pane moves and floating. */ -function TabHost({ state, callbacks, intents, tab, pane, column, floats, focusRequest, keepMounted, active = true }: TabHostProps): ReactNode { +function TabHost({ state, callbacks, intents, tab, pane, column, floats, focusRequest, keepMounted, + active = true }: TabHostProps): ReactNode { const floating = pane.host === 'float' const selected = floating || pane.activeTabId === tab.id const visible = active && selected && (floating || state.expanded) @@ -77,32 +78,32 @@ function TabHost({ state, callbacks, intents, tab, pane, column, floats, focusRe ) } @@ -118,7 +119,7 @@ export function TabLayout(props: LayoutProps): ReactNode { const focusRequest = useRef<{ readonly tabId: TabId; readonly origin: Element | null }>() const tabCallbacks: PaneCallbacks = { ...callbacks, - onFocusTab: tabId => { + onFocusTab: (tabId) => { // Only changing the selected body replaces its strip; pane-only focus does not. focusRequest.current = findTabPane(state, tabId).activeTabId === tabId ? undefined : { tabId, origin: document.activeElement } @@ -137,7 +138,7 @@ export function TabLayout(props: LayoutProps): ReactNode {
{/* Logical order changes must not make React move a connected webview's ancestor. */} - {Object.values(state.tabs).sort((a, b) => a.id.localeCompare(b.id)).map(tab => { + {Object.values(state.tabs).sort((a, b) => a.id.localeCompare(b.id)).map((tab) => { const pane = findTabPane(state, tab.id) return candidate.id === pane.id)} floats={floats} focusRequest={focusRequest} /> @@ -149,7 +150,7 @@ export function TabLayout(props: LayoutProps): ReactNode { ))} {root.kind === 'split' &&
{ callbacks.onDividerPressed(root.id, 0, event) }} />} + onPointerDown={(event) => { callbacks.onDividerPressed(root.id, 0, event) }} />}
) } diff --git a/packages/client/ui-dockkit/src/components/TabPanel.tsx b/packages/client/ui-dockkit/src/components/TabPanel.tsx index 9c31c43fbe..1088e71399 100644 --- a/packages/client/ui-dockkit/src/components/TabPanel.tsx +++ b/packages/client/ui-dockkit/src/components/TabPanel.tsx @@ -238,158 +238,158 @@ export function TabStrip({ state, pane, callbacks }: TabPanelProps): ReactNode { } return ( -
-
- {pane.tabs.map((tabId, index) => { - const tab = getTab(state, tabId) - const selected = tabId === pane.activeTabId - const closable = callbacks.canCloseTab(tabId) - // A pane's lone unclosable chip is a label, not a choice: there is - // no other tab to select against and nothing to do to it. - const quiet = !closable && pane.tabs.length === 1 - return ( - - {(index > 0 || stripIndex === index) && ( -
+
+ {pane.tabs.map((tabId, index) => { + const tab = getTab(state, tabId) + const selected = tabId === pane.activeTabId + const closable = callbacks.canCloseTab(tabId) + // A pane's lone unclosable chip is a label, not a choice: there is + // no other tab to select against and nothing to do to it. + const quiet = !closable && pane.tabs.length === 1 + return ( + + {(index > 0 || stripIndex === index) && ( +
+ )} +
{ + if (element === null) chips.delete(tabId) + else chips.set(tabId, element) + }} + // Focus lands on click, not on press: a state change between + // pointerdown and the first pointermove rebuilds this subtree, + // and Chromium cancels the pointer when the pressed element is + // replaced — which would abandon every drag. A drag that ends + // elsewhere fires no click, and its own operation carries focus. + onPointerDown={(event) => { + // A secondary press is the menu, never a drag. + if (event.button === 2) return + callbacks.onTabPressed(tabId, event) + }} + onClick={(event) => { + event.stopPropagation() + activate(tabId) + }} + onKeyDown={(event) => { + // Keys on the chip's nested close control are that control's. + if (event.target !== event.currentTarget) return + const next = chipToFocus(event.key, pane.tabs, tabId) + if (next !== undefined) { + event.preventDefault() + focusChip(next) + return + } + if (selects(event.key)) { + event.preventDefault() + activate(tabId) + } + }} + onContextMenu={(event) => { + event.preventDefault() + const anchor = event.currentTarget + setMenu(current => current?.tabId === tabId ? undefined : { tabId, anchor }) + }} + > + {callbacks.renderTabTitle?.(tab) ?? tab.title} + {closable && ( + + )} + {menu?.tabId === tabId && ( + { setMenu(undefined); callbacks.onCloseTab(tabId) } : undefined} + onDismiss={() => { setMenu(undefined) }} + extras={callbacks.renderTabMenuItems?.(tab, () => { setMenu(undefined) })} /> )} -
{ - if (element === null) chips.delete(tabId) - else chips.set(tabId, element) - }} - // Focus lands on click, not on press: a state change between - // pointerdown and the first pointermove rebuilds this subtree, - // and Chromium cancels the pointer when the pressed element is - // replaced — which would abandon every drag. A drag that ends - // elsewhere fires no click, and its own operation carries focus. - onPointerDown={(event) => { - // A secondary press is the menu, never a drag. - if (event.button === 2) return - callbacks.onTabPressed(tabId, event) - }} - onClick={(event) => { - event.stopPropagation() - activate(tabId) - }} - onKeyDown={(event) => { - // Keys on the chip's nested close control are that control's. - if (event.target !== event.currentTarget) return - const next = chipToFocus(event.key, pane.tabs, tabId) - if (next !== undefined) { - event.preventDefault() - focusChip(next) - return - } - if (selects(event.key)) { - event.preventDefault() - activate(tabId) - } - }} - onContextMenu={(event) => { - event.preventDefault() - const anchor = event.currentTarget - setMenu(current => current?.tabId === tabId ? undefined : { tabId, anchor }) - }} - > - {callbacks.renderTabTitle?.(tab) ?? tab.title} - {closable && ( - - )} - {menu?.tabId === tabId && ( - { setMenu(undefined); callbacks.onCloseTab(tabId) } : undefined} - onDismiss={() => { setMenu(undefined) }} - extras={callbacks.renderTabMenuItems?.(tab, () => { setMenu(undefined) })} - /> - )} -
- - ) - })} - {stripIndex === pane.tabs.length &&
} -
- {callbacks.canAddTab(pane.id) && ( - - - - )} -
- {!(callbacks.hideSplitWhenBlocked && block !== undefined) && ( - - - - )} - {/* The embedder's surface-wide controls, in the top-right pane only: the - strip is the surface's top edge, and this pane's end is its corner. */} - {pane.id === callbacks.chromePaneId && callbacks.chrome !== undefined && ( - // The embedder's controls report their own intents; the pane's - // click-to-focus must not add a focus entry to each of them. -
{ event.stopPropagation() }} - > - {callbacks.chrome} -
- )} +
+ + ) + })} + {stripIndex === pane.tabs.length &&
}
+ {callbacks.canAddTab(pane.id) && ( + + + + )} +
+ {!(callbacks.hideSplitWhenBlocked && block !== undefined) && ( + + + + )} + {/* The embedder's surface-wide controls, in the top-right pane only: the + strip is the surface's top edge, and this pane's end is its corner. */} + {pane.id === callbacks.chromePaneId && callbacks.chrome !== undefined && ( + // The embedder's controls report their own intents; the pane's + // click-to-focus must not add a focus entry to each of them. +
{ event.stopPropagation() }} + > + {callbacks.chrome} +
+ )} +
) } diff --git a/packages/client/ui-dockkit/tests/tab-layout.client.spec.tsx b/packages/client/ui-dockkit/tests/tab-layout.client.spec.tsx new file mode 100644 index 0000000000..7874de6c37 --- /dev/null +++ b/packages/client/ui-dockkit/tests/tab-layout.client.spec.tsx @@ -0,0 +1,162 @@ +// @vitest-environment jsdom +import { afterEach, expect, it, vi } from 'vitest' +import { act, cleanup, fireEvent, render } from '@testing-library/react' +import type { ComponentProps } from 'react' +import { TabLayout } from '../src/components/TabLayout.tsx' +import { DockController } from '../src/engine/controller.ts' +import { getNode, getPane } from '../src/engine/tree.ts' +import type { PaneCallbacks } from '../src/components/render.ts' +import type { TabId } from '../src/contract/types.ts' +import { followPointer } from '../src/components/pointer.ts' +import { seededController, TEST_LABELS } from './fixtures.client.ts' + +afterEach(() => { cleanup(); vi.restoreAllMocks() }) + +function mounted(controller = seededController(), options: Partial> = {}) { + const callbacks: PaneCallbacks = { + onFocusTab: (id) => { controller.focusTab(id); redraw() }, + onFocusPane: (id) => { controller.focusPane(id); redraw() }, + onSplitPane: vi.fn(), onAddTab: vi.fn(), onCloseTab: vi.fn(), onTabPressed: vi.fn(), onDividerPressed: vi.fn(), + splitBlock: () => undefined, canAddTab: () => true, canCloseTab: () => true, + dropTarget: undefined, draggingTabId: undefined, labels: TEST_LABELS, + renderTab: tab => , + renderTabTitle: undefined, renderTabMenuItems: undefined, + chromePaneId: getPane(controller.getSnapshot().state, controller.getSnapshot().state.rootId).id, + chrome: undefined, + } + const props = () => ({ state: controller.getSnapshot().state, callbacks, intents: controller, preview: undefined, ...options }) + const view = render() + function redraw(next: typeof options = {}) { + options = { ...options, ...next } + view.rerender() + } + const node = (selector: string): HTMLElement => { + const element = view.container.querySelector(selector) + if (element === null) throw new Error(`missing ${selector}`) + return element + } + return { controller, view, redraw, callbacks, node, body: (id: TabId) => node(`[data-body="${id}"]`) } +} + +it('retains visited bodies and hands keyboard focus to the newly selected strip', () => { + const controller = seededController() + controller.setExpanded(true) + const first = getPane(controller.getSnapshot().state, controller.getSnapshot().state.rootId).tabs[0]! + const second = controller.openContent({ kind: 'file', contentId: 'file:b', title: 'B' }) + controller.focusTab(first) + const h = mounted(controller, { keepMounted: () => true }) + const original = h.body(first) + expect(h.view.container.querySelector(`[data-body="${second}"]`)).toBeNull() + const chip = h.node(`[data-dockkit-tab="${second}"]`) + chip.focus() + fireEvent.click(chip) + expect(document.activeElement).toBe(h.node(`[data-dockkit-content="${second}"] [data-dockkit-tab="${second}"]`)) + expect(h.body(first)).toBe(original) + expect(original.closest('[data-dockkit-host]')).toHaveProperty('hidden', true) + fireEvent.click(h.node(`[data-dockkit-content="${second}"] [data-dockkit-tab="${second}"]`)) + const body = h.body(second) + body.focus() + h.redraw({ active: false }) + expect(body.closest('[data-dockkit-content]')).toHaveProperty('inert', true) + expect(document.activeElement).not.toBe(body) + fireEvent.focus(body) + h.redraw({ active: true }) + expect(h.body(second)).toBe(body) + expect(body.closest('[data-dockkit-content]')).toHaveProperty('inert', false) + act(() => { controller.setExpanded(false); h.redraw() }) + expect(h.body(second)).toBe(body) +}) + +it('keeps body ancestors through pane moves, floating, raising and docking', () => { + const controller = seededController() + controller.setExpanded(true) + const tab = controller.openContent({ kind: 'file', contentId: 'file:a', title: 'A' }) + const h = mounted(controller, { keepMounted: () => true }) + const body = h.body(tab) + const host = body.closest('[data-dockkit-host]')! + act(() => { controller.splitPane(); h.redraw() }) + expect(h.view.container.querySelectorAll('[data-dockkit-pane]')).toHaveLength(2) + const state = controller.getSnapshot().state + const split = getNode(state, state.rootId) + if (split.kind !== 'split') throw new Error('expected a split') + h.redraw({ preview: { splitId: split.id, sizes: [0.3, 0.7] } }) + expect(h.node('[data-dockkit-split]').style.gridTemplateColumns).toContain('0.3fr') + fireEvent.pointerDown(h.node('[data-dockkit-divider]'), { button: 0, pointerId: 1 }) + expect(h.callbacks.onDividerPressed).toHaveBeenCalledOnce() + h.redraw({ preview: undefined }) + fireEvent.focus(body) + expect(controller.getSnapshot().state.activePaneId).toBe(getPane(state, split.children[0]!).id) + const secondPane = getPane(state, split.children[1]!).id + act(() => { controller.placeTab(tab, secondPane, 0); h.redraw() }) + expect(h.body(tab)).toBe(body) + expect(body.closest('[data-dockkit-host]')).toBe(host) + act(() => { controller.floatTab(tab); h.redraw() }) + const floating = controller.getSnapshot().state.floats[0]! + expect(body.closest('[data-dockkit-host]')).toBe(host) + expect(host.getAttribute('data-dockkit-host')).toBe('float') + act(() => { controller.setExpanded(false); h.redraw() }) + expect(body.closest('[data-dockkit-content]')?.getAttribute('aria-hidden')).toBeNull() + fireEvent.pointerDown(h.node(`[data-dockkit-float="${floating}"]`), { pointerId: 1, button: 0 }) + fireEvent.focus(body) + const resize = h.node('[data-dockkit-float-resize]') + fireEvent.pointerDown(resize, { pointerId: 2, button: 0, clientX: 500, clientY: 400 }) + fireEvent.pointerMove(window, { pointerId: 2, clientX: 540, clientY: 440 }) + fireEvent.pointerUp(window, { pointerId: 2, clientX: 540, clientY: 440 }) + act(() => { h.redraw(); controller.unfloatPane(floating); h.redraw() }) + expect(h.body(tab)).toBe(body) + expect(body.closest('[data-dockkit-host]')).toBe(host) +}) + +it('unmounts unretained bodies and restores focus to their replacement strip', () => { + const controller = seededController() + controller.setExpanded(true) + const first = getPane(controller.getSnapshot().state, controller.getSnapshot().state.rootId).tabs[0]! + const second = controller.openContent({ kind: 'file', contentId: 'file:b', title: 'B' }) + controller.focusTab(first) + const h = mounted(controller) + const original = h.body(first) + fireEvent.click(h.node(`[data-dockkit-tab="${second}"]`)) + expect(original.isConnected).toBe(false) + h.redraw({ active: false }) + expect(h.view.container.querySelector('[data-body]')).toBeNull() + h.redraw({ active: true }) + const focus = vi.spyOn(HTMLElement.prototype, 'focus') + fireEvent.click(h.node(`[data-dockkit-tab="${first}"]`)) + expect(focus).toHaveBeenCalled() +}) + +it('renders an empty pane and rejects unsupported docked trees', () => { + const controller = new DockController() + controller.setExpanded(true) + const h = mounted(controller) + expect(h.node('[data-dockkit-empty]')).toBeDefined() + controller.openContent({ kind: 'file', contentId: 'file:a', title: 'A' }) + controller.splitPane() + const state = controller.getSnapshot().state + const root = getNode(state, state.rootId) + if (root.kind !== 'split') throw new Error('expected a split') + vi.spyOn(console, 'error').mockImplementation(() => {}) + const suppress = (event: ErrorEvent): void => { + if (event.error instanceof Error && event.error.message === 'DockLayout requires one pane or two horizontally split panes') { + event.preventDefault() + } + } + window.addEventListener('error', suppress) + try { + expect(() =>{ h.redraw({ state: { ...state, nodes: { ...state.nodes, [root.id]: { ...root, axis: 'column' } } } }) }) + .toThrow('two horizontally split panes') + } finally { + window.removeEventListener('error', suppress) + } +}) + +it('keeps a superseding gesture marker when an older follower detaches', () => { + const element = document.createElement('div') + const followers = { move: vi.fn(), up: vi.fn(), cancel: vi.fn() } + const first = followPointer(element, 1, followers) + const second = followPointer(element, 2, followers) + first() + expect(element.dataset.dockkitPointer).toBe('2') + second() + expect(element.dataset.dockkitPointer).toBeUndefined() +}) diff --git a/packages/client/ui-sidebar-browser/src/client/browser/BrowserController.ts b/packages/client/ui-sidebar-browser/src/client/browser/BrowserController.ts index 6911f88a56..521bd0d761 100644 --- a/packages/client/ui-sidebar-browser/src/client/browser/BrowserController.ts +++ b/packages/client/ui-sidebar-browser/src/client/browser/BrowserController.ts @@ -46,12 +46,12 @@ export class BrowserController implements HostObservable this.checkpoint = options.initial this.page = options.createPage({ initial: options.initial, - persist: state => { + persist: (state) => { if (this.disposed) return this.checkpoint = state this.actions.replace(options.tabId, state) }, - openRequested: value => { + openRequested: (value) => { if (this.disposed) return const result = parseBrowserAddress(value, options.applicationOrigin) if (!result.ok) { this.addressFailed(result.reason); return } @@ -254,15 +254,15 @@ export function createBrowserControllers(actions: BoundActions, cr controllers.clear() await Promise.all(pending) }, - rebind: actions => { + rebind: (actions) => { currentActions = actions for (const { controller } of controllers.values()) controller.rebind(actions) }, loadUrl: (id, value) => { controller(id)?.loadUrl(value) }, - restore: id => { controller(id)?.restore() }, - goBack: id => { controller(id)?.goBack() }, - goForward: id => { controller(id)?.goForward() }, - reload: id => { controller(id)?.reload() }, + restore: (id) => { controller(id)?.restore() }, + goBack: (id) => { controller(id)?.goBack() }, + goForward: (id) => { controller(id)?.goForward() }, + reload: (id) => { controller(id)?.reload() }, setSandbox: (id, enabled) => { controller(id)?.setSandbox(enabled) }, } } diff --git a/packages/client/ui-sidebar-browser/src/client/browser/ElectronWebViewImpl.ts b/packages/client/ui-sidebar-browser/src/client/browser/ElectronWebViewImpl.ts index 5307b7d8d2..79c075aaaf 100644 --- a/packages/client/ui-sidebar-browser/src/client/browser/ElectronWebViewImpl.ts +++ b/packages/client/ui-sidebar-browser/src/client/browser/ElectronWebViewImpl.ts @@ -145,13 +145,14 @@ export class ElectronWebViewImpl implements BrowserFrame { this.workspaceKey ??= await this.workspace(attachmentSignal) if (attachmentSignal.aborted) return const reservation = await this.bridge.acquire(this.workspaceKey) + // oxlint-disable-next-line typescript/no-unnecessary-condition -- The signal can abort while acquire is pending. if (attachmentSignal.aborted) { await this.release(reservation.lease); return } this.lease = reservation.lease this.guestLifetime = new AbortController() const signal = AbortSignal.any([attachmentSignal, this.guestLifetime.signal]) const element = this.presentation.createElement(reservation) this.element = element - const unsubscribeOpen = this.bridge.onOpenRequested(reservation.lease, url => { + const unsubscribeOpen = this.bridge.onOpenRequested(reservation.lease, (url) => { if (this.element === element && !signal.aborted) this.options.openRequested(url) }) signal.addEventListener('abort', unsubscribeOpen, { once: true }) @@ -161,10 +162,10 @@ export class ElectronWebViewImpl implements BrowserFrame { this.loadPending() }, { signal }) element.addEventListener('did-navigate', () => { this.observe(true) }, { signal }) - element.addEventListener('did-navigate-in-page', event => { + element.addEventListener('did-navigate-in-page', (event) => { if ((event as NavigationEvent).isMainFrame) this.observe(true) }, { signal }) - element.addEventListener('did-start-navigation', event => { + element.addEventListener('did-start-navigation', (event) => { if ((event as NavigationEvent).isMainFrame) { this.store.set({ ...this.store.getSnapshot(), loading: true, error: undefined }) } @@ -174,7 +175,7 @@ export class ElectronWebViewImpl implements BrowserFrame { this.observe(name === 'page-title-updated' && this.store.getSnapshot().address === 'observed') }, { signal }) } - element.addEventListener('did-fail-load', event => { + element.addEventListener('did-fail-load', (event) => { const failure = event as LoadFailureEvent if (failure.isMainFrame && failure.errorCode !== -3) { this.failed({ code: failure.errorCode, description: failure.errorDescription }) diff --git a/packages/client/ui-sidebar-browser/src/client/browser/IframeImpl.ts b/packages/client/ui-sidebar-browser/src/client/browser/IframeImpl.ts index da61877673..2de462ef1e 100644 --- a/packages/client/ui-sidebar-browser/src/client/browser/IframeImpl.ts +++ b/packages/client/ui-sidebar-browser/src/client/browser/IframeImpl.ts @@ -8,7 +8,7 @@ import type { BrowserTarget } from './url.ts' /** Owns iframe navigation; the view reports loads without reading cross-origin content. */ export class IframeImpl implements BrowserFrame { - readonly sandbox: BrowserSandboxControl = { setEnabled: enabled => { this.setSandbox(enabled) } } + readonly sandbox: BrowserSandboxControl = { setEnabled: (enabled) => { this.setSandbox(enabled) } } private readonly navigation: BrowserNavigation private readonly store: SnapshotStore private sandboxed = true diff --git a/packages/client/ui-sidebar-browser/src/client/browser/workspace.ts b/packages/client/ui-sidebar-browser/src/client/browser/workspace.ts index 0341de1c92..9298529a33 100644 --- a/packages/client/ui-sidebar-browser/src/client/browser/workspace.ts +++ b/packages/client/ui-sidebar-browser/src/client/browser/workspace.ts @@ -12,7 +12,12 @@ export async function browserWorkspace(source: WorkspaceSource, sessionId: strin signal.throwIfAborted() if (source.getSnapshot().phase !== 'ready') { await new Promise((resolve, reject) => { - const abort = (): void => { stop(); reject(signal.reason) } + const abort = (): void => { + stop() + const reason: unknown = signal.reason + // oxlint-disable-next-line typescript/prefer-promise-reject-errors -- Preserve the caller-owned AbortSignal reason. + reject(reason) + } const stop = source.subscribe(() => { if (source.getSnapshot().phase !== 'ready') return stop() diff --git a/packages/client/ui-sidebar-browser/src/client/index.ts b/packages/client/ui-sidebar-browser/src/client/index.ts index bc1be9ae18..af5eab39cc 100644 --- a/packages/client/ui-sidebar-browser/src/client/index.ts +++ b/packages/client/ui-sidebar-browser/src/client/index.ts @@ -72,7 +72,7 @@ export function apply(ctx: Context): void { }, BrowserBody)), 'ui-sidebar-browser.body') } if (desktop === undefined) installFrames(ctx, () => createIframePage) - else ctx.inject(['workspaces'], scope => { + else ctx.inject(['workspaces'], (scope) => { installFrames(scope, sessionId => options => createElectronPage(options, desktop, signal => browserWorkspace(scope.workspaces.list, sessionId, signal))) }) diff --git a/packages/client/ui-sidebar-browser/src/client/pages.ts b/packages/client/ui-sidebar-browser/src/client/pages.ts index 5f0df3d76a..b3ec7ef9c5 100644 --- a/packages/client/ui-sidebar-browser/src/client/pages.ts +++ b/packages/client/ui-sidebar-browser/src/client/pages.ts @@ -13,8 +13,8 @@ import { ElectronWebviewPresentation } from './view/ElectronWebviewPresentation. */ export function createIframePage(options: BrowserPageOptions): BrowserPage { const presentation = new IframePresentation({ - loaded: revision => { frame.handleLoaded(revision) }, - failed: revision => { frame.handleLoadFailed(revision) }, + loaded: (revision) => { frame.handleLoaded(revision) }, + failed: (revision) => { frame.handleLoadFailed(revision) }, remounted: () => { frame.reload() }, }) const frame = new IframeImpl(options, presentation) @@ -30,11 +30,10 @@ export function createIframePage(options: BrowserPageOptions): BrowserPage { */ export function createElectronPage(options: BrowserPageOptions, bridge: DesktopBrowserBridge, workspace: (signal: AbortSignal) => Promise): BrowserPage { - let frame: ElectronWebViewImpl const presentation = new ElectronWebviewPresentation({ mounted: () => { frame.attach() }, unmounted: () => { frame.detach() }, }) - frame = new ElectronWebViewImpl(options, bridge, workspace, presentation) + const frame = new ElectronWebViewImpl(options, bridge, workspace, presentation) return { frame, presentation } } diff --git a/packages/client/ui-sidebar-browser/src/client/view/BrowserBody.tsx b/packages/client/ui-sidebar-browser/src/client/view/BrowserBody.tsx index 7c2e4462f5..9ea53f03eb 100644 --- a/packages/client/ui-sidebar-browser/src/client/view/BrowserBody.tsx +++ b/packages/client/ui-sidebar-browser/src/client/view/BrowserBody.tsx @@ -40,7 +40,7 @@ export type BrowserBodyProps = PropsRuntime<'sidebar.right.pane.tab'> function useBrowserDraft(url: string | undefined, revision: number): readonly [string, (value: string) => void] { const [edit, setEdit] = useState<{ readonly revision: number; readonly value: string }>() return [edit?.revision === revision ? edit.value : url ?? '', - value => { setEdit({ revision, value }) }] + (value) => { setEdit({ revision, value }) }] } /** Render provider-neutral navigation state and optional controls. */ @@ -62,7 +62,7 @@ export function BrowserBody(props: BrowserBodyProps): ReactNode { const hide = mount({ tabId: tab.id, signal: tab.signal, viewportId, applicationOrigin: window.location.origin, initial: initial.current, initialUrl: initialUrl.current, - openTab: url => { tab.actions.openTab('browser', { params: { url }, revealIfOpened: false }) }, + openTab: (url) => { tab.actions.openTab('browser', { params: { url }, revealIfOpened: false }) }, }) setMountEpoch(value => value + 1) return hide @@ -88,7 +88,7 @@ export function BrowserBody(props: BrowserBodyProps): ReactNode { aria-label={t('address.placeholder')} placeholder={t('address.placeholder')} spellCheck={false} - onChange={event => { setDraft(event.currentTarget.value) }} + onChange={(event) => { setDraft(event.currentTarget.value) }} /> {unknown && {t('address.changed')}} diff --git a/packages/client/ui-sidebar-browser/tests/browser-body.client.spec.tsx b/packages/client/ui-sidebar-browser/tests/browser-body.client.spec.tsx index 952b8cd4f7..a062a29a15 100644 --- a/packages/client/ui-sidebar-browser/tests/browser-body.client.spec.tsx +++ b/packages/client/ui-sidebar-browser/tests/browser-body.client.spec.tsx @@ -43,28 +43,28 @@ function mountBrowser(navigation?: { readonly url?: string }) { const tabActions = { openResource: vi.fn(), openTab: vi.fn(), close: vi.fn() } const props: Pick & Omit = { - sessionId: SESSION, - useTabInfo: () => ({ - sidebar: { expanded: true, fullscreen: false }, panel: { id: 'pane' as PaneId }, - tab: { - id: TAB, kind: 'browser', title: 'Browser', contentId: 'sidebar://browser/1', visible: true, - navigation: { address: 'sidebar://browser/1', params: navigation, revision: 0 }, - signal: lifetime.signal, - actions: tabActions, + sessionId: SESSION, + useTabInfo: () => ({ + sidebar: { expanded: true, fullscreen: false }, panel: { id: 'pane' as PaneId }, + tab: { + id: TAB, kind: 'browser', title: 'Browser', contentId: 'sidebar://browser/1', visible: true, + navigation: { address: 'sidebar://browser/1', params: navigation, revision: 0 }, + signal: lifetime.signal, + actions: tabActions, + }, + }), + useStore: hookOf(store), + actions: store.actions, + t: (key, params) => { + const template = messages[key] ?? key + return params === undefined ? template : template.replace(/\{(\w+)\}/g, (_match, name: string) => String(params[name])) }, - }), - useStore: hookOf(store), - actions: store.actions, - t: (key, params) => { - const template = messages[key] ?? key - return params === undefined ? template : template.replace(/\{(\w+)\}/g, (_match, name: string) => String(params[name])) - }, - ...commands, - useBrowserState: (key: string) => { - const state = keyedHooks.browserState(key) ?? absentState - return useSyncExternalStore(state.subscribe, state.getSnapshot) - }, - } + ...commands, + useBrowserState: (key: string) => { + const state = keyedHooks.browserState(key) ?? absentState + return useSyncExternalStore(state.subscribe, state.getSnapshot) + }, + } const renderBody = () => render() return { view: renderBody(), remount: renderBody, store, lifetime, injected, diff --git a/packages/client/ui-sidebar-browser/tests/electron-frame.client.spec.ts b/packages/client/ui-sidebar-browser/tests/electron-frame.client.spec.ts index 75b6cf3d09..53a54ea551 100644 --- a/packages/client/ui-sidebar-browser/tests/electron-frame.client.spec.ts +++ b/packages/client/ui-sidebar-browser/tests/electron-frame.client.spec.ts @@ -16,7 +16,7 @@ it('clears a failed load when the main page retries without a toolbar command', acquire: vi.fn(async () => ({ lease: 'lease' as DesktopBrowserLeaseId, partition: 'partition' })), release: vi.fn(async () => {}), onOpenRequested: () => () => {}, } - const presentation = new ElectronWebviewPresentation({ mounted: () => frame.attach(), unmounted: () => frame.detach() }) + const presentation = new ElectronWebviewPresentation({ mounted: () =>{ frame.attach() }, unmounted: () =>{ frame.detach() } }) const frame = new ElectronWebViewImpl({ initial: undefined, persist: vi.fn(), openRequested: vi.fn() }, bridge, async () => 'cwd:/workspace', presentation) const createElement = vi.spyOn(presentation, 'createElement').mockReturnValue(element) diff --git a/packages/client/ui-sidebar-browser/tests/electron-harness.client.ts b/packages/client/ui-sidebar-browser/tests/electron-harness.client.ts new file mode 100644 index 0000000000..31a8f521d5 --- /dev/null +++ b/packages/client/ui-sidebar-browser/tests/electron-harness.client.ts @@ -0,0 +1,66 @@ +/** Native webview events controlled by each test; presentation and navigation stay real. */ +import { vi } from 'vitest' +import type { DesktopBrowserBridge, DesktopBrowserLeaseId, DesktopBrowserReservation } from '../src/types.ts' +import type { BrowserTabState } from '../src/client/browser/BrowserPersistence.ts' +import { createElectronPage } from '../src/client/pages.ts' +import { ElectronWebviewPresentation } from '../src/client/view/ElectronWebviewPresentation.ts' + +let sequence = 0 + +/** @returns one isolated, explicitly mounted page with native operations replaced by spies. */ +export function electronFixture(initial?: BrowserTabState) { + const opens = new Set<(url: string) => void>() + const reservation: DesktopBrowserReservation = { lease: `lease-${++sequence}` as DesktopBrowserLeaseId, partition: 'partition' } + const bridge = { + acquire: vi.fn(async (_workspace: string) => reservation), + release: vi.fn(async (_lease: DesktopBrowserLeaseId) => {}), + onOpenRequested: vi.fn((_lease: DesktopBrowserLeaseId, listener: (url: string) => void) => { + opens.add(listener) + return () => { opens.delete(listener) } + }), + } satisfies DesktopBrowserBridge + const workspace = vi.fn(async (_signal: AbortSignal) => 'cwd:/workspace') + const persist = vi.fn() + const openRequested = vi.fn() + const page = createElectronPage({ initial, persist, openRequested }, bridge, workspace) + const presentation = page.presentation + if (!(presentation instanceof ElectronWebviewPresentation)) throw new Error('expected the Electron presentation') + const create = presentation.createElement.bind(presentation) + const guests: ReturnType[] = [] + function prepareGuest(approved: DesktopBrowserReservation) { + const element = create(approved) + const state = { url: 'about:blank', title: '', loading: true, back: false, forward: false } + const methods = { + loadURL: vi.fn(async (_url: string) => {}), getURL: vi.fn(() => state.url), getTitle: vi.fn(() => state.title), + canGoBack: () => state.back, canGoForward: () => state.forward, clearHistory: vi.fn(), + goBack: vi.fn(), goForward: vi.fn(), reload: vi.fn(), isLoading: () => state.loading, + } + Object.assign(element, methods) + const emit = (type: string, fields: object = {}): void => { element.dispatchEvent(Object.assign(new Event(type), fields)) } + return { element, state, emit, ...methods } + } + const createElement = vi.spyOn(presentation, 'createElement').mockImplementation((approved) => { + const guest = prepareGuest(approved) + guests.push(guest) + return guest.element + }) + const host = document.createElement('div') + host.id = `electron-fixture-${sequence}` + document.body.append(host) + return { + ...page, presentation, bridge, workspace, persist, openRequested, opens, guests, host, reservation, + mount: () => presentation.mount(host.id), + async guest() { + await vi.waitFor(() => { expectGuest() }) + return guests.at(-1)! + }, + async dispose() { + await page.frame.dispose() + createElement.mockRestore() + host.remove() + }, + } + function expectGuest(): void { + if (host.firstElementChild === null || guests.length === 0) throw new Error('guest has not attached') + } +} diff --git a/packages/client/ui-sidebar-browser/tests/electron-lifecycle.client.spec.ts b/packages/client/ui-sidebar-browser/tests/electron-lifecycle.client.spec.ts new file mode 100644 index 0000000000..ef14e6de07 --- /dev/null +++ b/packages/client/ui-sidebar-browser/tests/electron-lifecycle.client.spec.ts @@ -0,0 +1,179 @@ +// @vitest-environment jsdom +import { afterEach, expect, it, vi } from 'vitest' +import type { DesktopBrowserReservation } from '../src/types.ts' +import { electronFixture } from './electron-harness.client.ts' + +const target = { kind: 'https' as const, url: 'https://example.test/', title: 'Example' } +const fixtures: ReturnType[] = [] +function fixture() { + const h = electronFixture() + fixtures.push(h) + return h +} +afterEach(async () => { + for (const h of fixtures.splice(0)) await h.dispose() + vi.restoreAllMocks() +}) + +it('waits for a mounted container and observes native history, titles and new-tab requests', async () => { + const h = fixture() + h.frame.goBack() + h.frame.goForward() + h.frame.reload() + h.frame.loadUrl(target) + expect(h.bridge.acquire).not.toHaveBeenCalled() + h.mount() + const guest = await h.guest() + expect(guest.element.getAttribute('partition')).toBe(h.reservation.partition) + expect(guest.element.getAttribute('src')).toBe(`about:blank#${h.reservation.lease}`) + guest.emit('dom-ready') + expect(guest.loadURL).toHaveBeenCalledWith(target.url) + guest.state.url = target.url + guest.state.title = 'Title' + guest.state.back = true + guest.state.forward = true + guest.emit('did-navigate') + expect(guest.clearHistory).toHaveBeenCalledOnce() + expect(h.frame.getSnapshot()).toMatchObject({ address: 'observed', target: { ...target, title: 'Title' } }) + expect(guest.element.getAttribute('aria-label')).toBe('Title') + const writes = h.persist.mock.calls.length + guest.emit('page-title-updated') + expect(h.persist).toHaveBeenCalledTimes(writes) + h.frame.loadUrl(target) + h.frame.goBack() + h.frame.goForward() + expect(guest.reload).toHaveBeenCalledOnce() + expect(guest.goBack).toHaveBeenCalledOnce() + expect(guest.goForward).toHaveBeenCalledOnce() + guest.state.url = `${target.url}next` + guest.emit('did-navigate-in-page', { isMainFrame: false }) + expect(h.frame.getSnapshot().target?.url).toBe(target.url) + guest.emit('did-navigate-in-page', { isMainFrame: true }) + guest.state.title = '' + guest.state.loading = false + guest.emit('page-title-updated') + guest.emit('did-stop-loading') + expect(h.frame.getSnapshot()).toMatchObject({ loading: false, target: { url: guest.state.url, title: 'example.test' } }) + const open = h.bridge.onOpenRequested.mock.calls[0]![1] + open('https://new.example/') + expect(h.openRequested).toHaveBeenCalledOnce() + await h.frame.dispose() + open('https://late.example/') + guest.emit('did-navigate') + h.frame.loadUrl(target) + h.frame.reload() + h.frame.goBack() + expect(h.openRequested).toHaveBeenCalledOnce() + expect(h.opens.size).toBe(0) + expect(h.bridge.release).toHaveBeenCalledExactlyOnceWith(h.reservation.lease) +}) + +it('releases an acquisition that finishes after attachment cancellation', async () => { + const h = fixture() + const pending = Promise.withResolvers() + h.bridge.acquire.mockReturnValueOnce(pending.promise) + const unmount = h.mount() + h.frame.loadUrl(target) + try { + await vi.waitFor(() => { expect(h.bridge.acquire).toHaveBeenCalledOnce() }) + unmount() + const disposed = h.frame.dispose() + expect(h.frame.dispose()).toBe(disposed) + pending.resolve(h.reservation) + await disposed + expect(h.guests).toHaveLength(0) + expect(h.bridge.release).toHaveBeenCalledExactlyOnceWith(h.reservation.lease) + } finally { + pending.resolve(h.reservation) + } +}) + +it('does not acquire a guest when Workspace resolution finishes after disposal', async () => { + const h = fixture() + const pending = Promise.withResolvers() + h.workspace.mockReturnValueOnce(pending.promise) + h.mount() + h.frame.loadUrl(target) + try { + const disposed = h.frame.dispose() + pending.resolve('cwd:/late') + await disposed + expect(h.bridge.acquire).not.toHaveBeenCalled() + h.mount() + expect(h.guests).toHaveLength(0) + } finally { + pending.resolve('cwd:/late') + } +}) + +it('recreates the guest after physical remount or crash without resolving Workspace again', async () => { + const h = fixture() + const unmount = h.mount() + h.frame.loadUrl(target) + const first = await h.guest() + first.emit('dom-ready') + unmount() + h.mount() + const second = await h.guest() + expect(second.element).not.toBe(first.element) + expect(h.workspace).toHaveBeenCalledOnce() + second.emit('dom-ready') + expect(second.loadURL).toHaveBeenCalledWith(target.url) + second.emit('render-process-gone') + expect(h.frame.getSnapshot().error).toBeDefined() + h.frame.reload() + const third = await h.guest() + third.emit('dom-ready') + expect(third.loadURL).toHaveBeenCalledWith(target.url) + expect(h.frame.getSnapshot().error).toBeUndefined() + third.emit('destroyed') + expect(h.frame.getSnapshot().error).toBeDefined() +}) + +it('contains acquisition, native command and release failures and keeps retry available', async () => { + const h = fixture() + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + h.bridge.acquire.mockRejectedValueOnce(new Error('acquire failed')) + h.mount() + h.frame.loadUrl(target) + await vi.waitFor(() => { expect(h.frame.getSnapshot().error).toBeDefined() }) + h.frame.reload() + const guest = await h.guest() + guest.emit('dom-ready') + guest.state.url = target.url + guest.emit('did-navigate') + guest.reload.mockImplementationOnce(() => { throw new Error('command failed') }) + h.frame.reload() + expect(h.frame.getSnapshot().error).toBeDefined() + h.frame.reload() + expect(h.frame.getSnapshot().error).toBeUndefined() + guest.getURL.mockImplementationOnce(() => { throw new Error('observation failed') }) + guest.emit('did-navigate') + expect(h.frame.getSnapshot().error).toBeDefined() + h.bridge.release.mockRejectedValueOnce(new Error('release failed')) + await h.frame.dispose() + expect(error.mock.calls.some(call => call[0] === 'Desktop browser guest release failed')).toBe(true) +}) + +it('ignores superseded and aborted load promises, but publishes an active navigation rejection', async () => { + const h = fixture() + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + h.mount() + h.frame.loadUrl(target) + const guest = await h.guest() + const stale = Promise.withResolvers() + guest.loadURL.mockReturnValueOnce(stale.promise) + guest.emit('dom-ready') + h.frame.loadUrl({ ...target, url: 'https://example.test/new' }) + stale.reject(new Error('old navigation failed')) + await expect(stale.promise).rejects.toThrow('old navigation') + expect(h.frame.getSnapshot().error).toBeUndefined() + guest.loadURL.mockRejectedValueOnce(Object.assign(new Error('aborted'), { code: 'ERR_ABORTED' })) + h.frame.loadUrl({ ...target, url: 'https://example.test/cancelled' }) + await Promise.resolve() + expect(h.frame.getSnapshot().error).toBeUndefined() + guest.loadURL.mockRejectedValueOnce(new Error('active navigation failed')) + h.frame.loadUrl(target) + await vi.waitFor(() => { expect(h.frame.getSnapshot().error).toBeDefined() }) + expect(error).toHaveBeenCalledOnce() +}) diff --git a/packages/client/ui-sidebar-browser/tests/workspace.client.spec.ts b/packages/client/ui-sidebar-browser/tests/workspace.client.spec.ts new file mode 100644 index 0000000000..bd98e06d45 --- /dev/null +++ b/packages/client/ui-sidebar-browser/tests/workspace.client.spec.ts @@ -0,0 +1,57 @@ +import { afterEach, expect, it, vi } from 'vitest' +import { createSnapshotStore } from '@deepseek-ai/dsh-client-store' +import type { WorkspaceId, WorkspaceSnapshot, WorkspaceView } from '@deepseek-ai/dsh-api-workspace-controller/client' +import type { SessionId } from '@deepseek-ai/dsh-session/types' +import { browserWorkspace } from '../src/client/browser/workspace.ts' + +const SESSION = 'session' as SessionId +const workspace = (id: string, path: string, sessionIds: readonly SessionId[]): WorkspaceView => ({ + workspaceId: id as WorkspaceId, path, title: path, sessionIds, createdAt: '', updatedAt: '', +}) +const snapshot = (phase: WorkspaceSnapshot['phase'], items: readonly WorkspaceView[] = []): WorkspaceSnapshot => ({ + phase, items, state: 'idle', error: null, archivedSessionIds: [], pinnedSessionIds: [], +}) + +afterEach(() => { vi.restoreAllMocks() }) + +it('groups resolved Sessions by canonical CWD rather than Workspace identity', async () => { + const source = createSnapshotStore(snapshot('ready', [ + workspace('other', '/other', ['other' as SessionId]), workspace('first', '/canonical', [SESSION]), + ])) + const signal = new AbortController().signal + await expect(browserWorkspace(source, SESSION, signal)).resolves.toBe('cwd:/canonical') + source.set(snapshot('ready', [workspace('renamed', '/canonical', [SESSION])])) + await expect(browserWorkspace(source, SESSION, signal)).resolves.toBe('cwd:/canonical') + await expect(browserWorkspace(source, 'unaccounted', signal)).resolves.toBe('session:unaccounted') +}) + +it('waits for an authoritative baseline and removes the readiness subscription', async () => { + const source = createSnapshotStore(snapshot('pending')) + const subscribe = source.subscribe.bind(source) + const stopped = vi.fn() + vi.spyOn(source, 'subscribe').mockImplementation((listener) => { + const stop = subscribe(listener) + return () => { stop(); stopped() } + }) + const lifetime = new AbortController() + const result = browserWorkspace(source, SESSION, lifetime.signal) + source.set(snapshot('pending', [workspace('partial', '/partial', [SESSION])])) + expect(stopped).not.toHaveBeenCalled() + source.set(snapshot('ready', [workspace('ready', '/ready', [SESSION])])) + await expect(result).resolves.toBe('cwd:/ready') + expect(stopped).toHaveBeenCalledOnce() + lifetime.abort() + expect(stopped).toHaveBeenCalledOnce() +}) + +it('rejects cancellation before and during readiness without retaining a listener', async () => { + const source = createSnapshotStore(snapshot('pending')) + const lifetime = new AbortController() + const failure = new Error('tab closed') + const result = browserWorkspace(source, SESSION, lifetime.signal) + const rejected = expect(result).rejects.toBe(failure) + lifetime.abort(failure) + await rejected + source.set(snapshot('ready')) + await expect(browserWorkspace(source, SESSION, lifetime.signal)).rejects.toBe(failure) +}) diff --git a/packages/client/ui-sidebar-right/src/client/contract/slots.ts b/packages/client/ui-sidebar-right/src/client/contract/slots.ts index e4468cf80e..28b026a3ad 100644 --- a/packages/client/ui-sidebar-right/src/client/contract/slots.ts +++ b/packages/client/ui-sidebar-right/src/client/contract/slots.ts @@ -39,11 +39,15 @@ declare module '@deepseek-ai/dsh-client-ui-slots' { interface SlotMap { /** Session content selected by the root-scoped right Sidebar controller. */ - 'rightbar.session': { kind: 'single'; scope: 'session'; owner: RightbarOwnerProps & { - readonly active: boolean - /** @param tabId - retained body. @param signal - tab occurrence lifetime. @returns releases the View-owned hold. */ - readonly retainTab: (tabId: TabId, signal: AbortSignal) => () => void - } } + 'rightbar.session': { + kind: 'single' + scope: 'session' + owner: RightbarOwnerProps & { + readonly active: boolean + /** @param tabId - retained body. @param signal - tab occurrence lifetime. @returns releases the View-owned hold. */ + readonly retainTab: (tabId: TabId, signal: AbortSignal) => () => void + } + } /** * One tab's body, dispatched with the `id` of the type in force for * `tab.kind`. A tab type registers here under its definition's `id` and diff --git a/packages/client/ui-sidebar-right/src/client/session-view.ts b/packages/client/ui-sidebar-right/src/client/session-view.ts index 1a87112cbe..f36bb3f977 100644 --- a/packages/client/ui-sidebar-right/src/client/session-view.ts +++ b/packages/client/ui-sidebar-right/src/client/session-view.ts @@ -31,7 +31,7 @@ export class SidebarSessionView { private readonly onTabRelease: (view: SidebarSessionView) => void, ) { this.reference = sessions.retain(sessionId, { source: 'sidebarView' }) - void this.reference.ready.catch(error => { console.error('Sidebar Session opening failed:', error) }) + void this.reference.ready.catch((error: unknown) => { console.error('Sidebar Session opening failed:', error) }) } /** Whether an initialized retained body still needs this view. */ diff --git a/packages/client/ui-sidebar-right/src/client/session-views.ts b/packages/client/ui-sidebar-right/src/client/session-views.ts index 01c87cdb41..04f4122094 100644 --- a/packages/client/ui-sidebar-right/src/client/session-views.ts +++ b/packages/client/ui-sidebar-right/src/client/session-views.ts @@ -32,9 +32,9 @@ export class SidebarSessionViews { if (this.closed || this.selected === sessionId) return this.selected = sessionId if (sessionId !== undefined && !this.views.has(sessionId)) { - const view = new SidebarSessionView(sessionId, this.sessions, disposed => { + const view = new SidebarSessionView(sessionId, this.sessions, (disposed) => { this.viewsByReference.delete(disposed.reference) - }, released => { this.prune(released) }) + }, (released) => { this.prune(released) }) this.views.set(sessionId, view) this.viewsByReference.set(view.reference, view) } diff --git a/packages/client/ui-sidebar-right/src/client/shell/SidebarRight.tsx b/packages/client/ui-sidebar-right/src/client/shell/SidebarRight.tsx index ad19ee4a0d..38a86eff2f 100644 --- a/packages/client/ui-sidebar-right/src/client/shell/SidebarRight.tsx +++ b/packages/client/ui-sidebar-right/src/client/shell/SidebarRight.tsx @@ -421,7 +421,9 @@ export function RightbarSeat({ // Republished on every committed change: the service's readers answer from the // last commit, and its commands act on the session actually on screen. useEffect( - () => active ? bindService({ sessionId, actions, surfaces, canSplitPane: paneId => room.current.get(paneId)?.row !== false }) : undefined, + () => active + ? bindService({ sessionId, actions, surfaces, canSplitPane: paneId => room.current.get(paneId)?.row !== false }) + : undefined, [bindService, sessionId, actions, surfaces, active], ) // The Tab domain is not synced here: the controller adopted this session's diff --git a/packages/client/ui-sidebar-right/tests/session-views.client.spec.ts b/packages/client/ui-sidebar-right/tests/session-views.client.spec.ts new file mode 100644 index 0000000000..1c11e825a7 --- /dev/null +++ b/packages/client/ui-sidebar-right/tests/session-views.client.spec.ts @@ -0,0 +1,114 @@ +// @vitest-environment jsdom +import { afterEach, expect, it, vi } from 'vitest' +import { SlotTestRuntime } from '@deepseek-ai/dsh-client-test-runtime' +import type { SessionId } from '@deepseek-ai/dsh-session/types' +import type { TabId } from '@deepseek-ai/dsh-client-ui-dockkit' +import { SidebarSessionView } from '../src/client/session-view.ts' +import { SidebarSessionViews } from '../src/client/session-views.ts' + +const A = 'a' as SessionId +const B = 'b' as SessionId +const TAB = 'tab' as TabId +const runtimes: SlotTestRuntime[] = [] +const owners: { dispose(): void }[] = [] + +afterEach(async () => { + for (const owner of owners.splice(0)) owner.dispose() + for (const runtime of runtimes.splice(0)) await runtime.dispose() + vi.restoreAllMocks() +}) + +async function runtime() { + const runtime = await SlotTestRuntime.create() + runtimes.push(runtime) + await runtime.sessions.add({ id: A }) + await runtime.sessions.add({ id: B }) + return runtime +} + +it('keeps one reference per View until the final committed root of a retired View unmounts', async () => { + const r = await runtime() + const disposed = vi.fn() + const changed = vi.fn() + const view = new SidebarSessionView(A, r.sessions, disposed, changed) + owners.push(view) + await view.reference.ready + const release = vi.spyOn(view.reference, 'release') + const firstMount = view.mount() + const secondMount = view.mount() + const first = new AbortController() + const second = new AbortController() + const firstHold = view.retainTab(TAB, first.signal) + const secondHold = view.retainTab(TAB, second.signal) + firstHold() + firstHold() + expect(view.hasRetainedTabs).toBe(true) + expect(changed).toHaveBeenCalledOnce() + second.abort() + secondHold() + expect(view.hasRetainedTabs).toBe(false) + view.retainTab(TAB, second.signal)() + view.retire() + firstMount() + expect(release).not.toHaveBeenCalled() + secondMount() + expect(release).toHaveBeenCalledOnce() + view.retainTab(TAB, first.signal)() + view.dispose() + expect(disposed).toHaveBeenCalledExactlyOnceWith(view) +}) + +it('retires only unselected Views without retained bodies and rejects released reference identities', async () => { + const r = await runtime() + const views = new SidebarSessionViews(r.sessions) + owners.push(views) + views.select(A) + const first = views.source.getSnapshot() + const a = first[0]! + views.select(A) + expect(views.source.getSnapshot()).toBe(first) + const unmount = views.mount(a.reference) + const lifetime = new AbortController() + const release = vi.spyOn(a.reference, 'release') + a.retainTab(TAB, lifetime.signal) + views.select(B) + expect(views.source.getSnapshot().map(view => [view.sessionId, view.selected])).toEqual([[A, false], [B, true]]) + lifetime.abort() + expect(views.source.getSnapshot().map(view => view.sessionId)).toEqual([B]) + expect(release).not.toHaveBeenCalled() + unmount() + expect(release).toHaveBeenCalledOnce() + expect(() => views.mount(a.reference)).toThrow('no longer owned') + views.select(undefined) + expect(views.source.getSnapshot()).toEqual([]) + views.dispose() + views.select(A) + views.mount(a.reference)() + expect(views.source.getSnapshot()).toEqual([]) +}) + +it('reports opening failure and releases Views even when committed roots are still present', async () => { + const r = await runtime() + const reference = r.sessions.retain(A, { source: 'sidebarView' }) + const failure = new Error('opening failed') + const release = vi.fn(() => { reference.release() }) + vi.spyOn(r.sessions, 'retain').mockReturnValueOnce({ + sessionId: reference.sessionId, + get binding() { return reference.binding }, + ready: Promise.reject(failure), + release, + [Symbol.dispose]: () => { reference.release() }, + }) + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + const views = new SidebarSessionViews(r.sessions) + owners.push(views) + views.select(A) + const view = views.source.getSnapshot()[0]! + const unmount = views.mount(view.reference) + await expect(view.reference.ready).rejects.toBe(failure) + expect(error).toHaveBeenCalledWith('Sidebar Session opening failed:', failure) + views.dispose() + expect(release).toHaveBeenCalledOnce() + unmount() + expect(release).toHaveBeenCalledOnce() +}) diff --git a/packages/extensions/cordis-client-runner/src/client/slot-catalog.ts b/packages/extensions/cordis-client-runner/src/client/slot-catalog.ts index 567b0f5781..bb9b004503 100644 --- a/packages/extensions/cordis-client-runner/src/client/slot-catalog.ts +++ b/packages/extensions/cordis-client-runner/src/client/slot-catalog.ts @@ -2664,7 +2664,7 @@ export const CLIENT_SLOT_API: readonly ClientSlotEntry[] = [ ], replaceRisk: 'none', example: 'return {\n inject: [\'slots\'],\n apply(ctx) {\n ctx.slots.inject(\'sidebar.right.pane.tab\', () => ctx.slots.register(\n { name: \'sidebar.right.pane.tab\', key: \'\' },\n () => React.createElement(\'div\', null, \'hello\'),\n ))\n },\n}', - source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:54', + source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:58', }, { key: 'sidebar.right.pane.tab.title', @@ -2713,7 +2713,7 @@ export const CLIENT_SLOT_API: readonly ClientSlotEntry[] = [ ], replaceRisk: 'none', example: 'return {\n inject: [\'slots\'],\n apply(ctx) {\n ctx.slots.inject(\'sidebar.right.pane.tab.title\', () => ctx.slots.register(\n { name: \'sidebar.right.pane.tab.title\', key: \'\' },\n () => React.createElement(\'div\', null, \'hello\'),\n ))\n },\n}', - source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:68', + source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:72', }, { key: 'sidebar.right.tab.document', @@ -3017,7 +3017,7 @@ export const CLIENT_SLOT_API: readonly ClientSlotEntry[] = [ occupants: [], replaceRisk: 'none', example: 'return {\n inject: [\'slots\'],\n apply(ctx) {\n ctx.slots.inject(\'sidebar.right.tab.guide\', () => ctx.slots.register(\n { name: \'sidebar.right.tab.guide\', select: owner => null },\n () => React.createElement(\'div\', null, \'hello\'),\n ))\n },\n}', - source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:79', + source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:83', }, { key: 'sidebar.right.tab.guide.entry', @@ -3063,7 +3063,7 @@ export const CLIENT_SLOT_API: readonly ClientSlotEntry[] = [ ], replaceRisk: 'none', example: 'return {\n inject: [\'slots\'],\n apply(ctx) {\n ctx.slots.inject(\'sidebar.right.tab.guide.entry\', () => ctx.slots.register(\n { name: \'sidebar.right.tab.guide.entry\', key: \'\' },\n () => React.createElement(\'div\', null, \'hello\'),\n ))\n },\n}', - source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:86', + source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:90', }, { key: 'sidebar.right.tab.menu.item', @@ -3121,7 +3121,7 @@ export const CLIENT_SLOT_API: readonly ClientSlotEntry[] = [ occupants: [], replaceRisk: 'none', example: 'return {\n inject: [\'slots\'],\n apply(ctx) {\n ctx.slots.inject(\'sidebar.right.tab.menu.item\', () => ctx.slots.register(\n { name: \'sidebar.right.tab.menu.item\', id: \'my-entry\', order: 100, label: \'My entry\' },\n () => React.createElement(\'div\', null, \'hello\'),\n ))\n },\n}', - source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:98', + source: 'packages/client/ui-sidebar-right/src/client/contract/slots.ts:102', }, { key: 'sidebar.settings',