From 7114cc7475d4c3cb83bb556fc98e991b8c76a8ca Mon Sep 17 00:00:00 2001 From: creatixchu Date: Wed, 29 Jul 2026 02:36:08 +0800 Subject: [PATCH] fix(client): flow occupancy is a subscribed source; an unloading occupant withdraws the open flow hasDirectoryFlow was a plain per-render read: a flow plugin unloading (HMR) while its dialog was open left flowOpen stuck with nobody to cancel, permanently disabling the workspace actions. Occupancy now rides useSyncExternalStore over the hole's registration subscription, and an empty hole withdraws an active flow; the menu entry also reacts to activation without a reopen (ds-review-bot). --- .../src/client/WorkspaceBrowser.tsx | 2 + .../src/client/WorkspacePicker.tsx | 23 +++++--- .../ui-workspace/src/client/contract/slots.ts | 13 +++-- .../client/ui-workspace/src/client/index.ts | 2 + .../client/ui-workspace/tests/apply.spec.ts | 5 ++ .../tests/workspace-browser.spec.tsx | 1 + .../tests/workspace-picker.spec.tsx | 53 ++++++++++++++----- 7 files changed, 77 insertions(+), 22 deletions(-) diff --git a/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx b/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx index 61da0cc1ea..9e7eb89872 100644 --- a/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx +++ b/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx @@ -254,6 +254,7 @@ export function WorkspaceBrowser({ insertSessionBefore, createWorkspace, hasDirectoryFlow, + subscribeDirectoryFlow, renderSlot, }: WorkspaceBrowserProps) { const workspaces = useWorkspaces(state => state.items) @@ -373,6 +374,7 @@ export function WorkspaceBrowser({ useWorkspaces={useWorkspaces} createWorkspace={createWorkspace} hasDirectoryFlow={hasDirectoryFlow} + subscribeDirectoryFlow={subscribeDirectoryFlow} renderDirectoryFlow={owner => renderSlot('sidebar.workspaces.directoryFlow', owner)} createOnly side="right" diff --git a/packages/client/ui-workspace/src/client/WorkspacePicker.tsx b/packages/client/ui-workspace/src/client/WorkspacePicker.tsx index 83c06b369c..cc4989c463 100644 --- a/packages/client/ui-workspace/src/client/WorkspacePicker.tsx +++ b/packages/client/ui-workspace/src/client/WorkspacePicker.tsx @@ -7,7 +7,7 @@ * opens the flow, adopts the picked path, and owns the error surface. */ import type { ReactNode, RefObject } from 'react' -import { useCallback, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState, useSyncExternalStore } from 'react' import { Button, IconFolderClose16, IconPlusOutline16, Menu, Modal, type MenuEntry, } from '@deepseek-ai/dsh-client-ui-primitives' @@ -33,8 +33,10 @@ export interface WorkspaceCreateFlowProps { useWorkspaces: (selector: (state: WorkspaceListState) => S) => S /** Create or adopt a real Host Workspace. */ createWorkspace: (input: { name: string } | { path: string }) => Promise - /** Whether this surface's directory-flow hole is occupied (read per menu render; empty hides the local-folder entry). */ + /** Whether this surface's directory-flow hole is occupied (empty hides the local-folder entry). */ hasDirectoryFlow: () => boolean + /** Registration-change subscription for the same hole (the uSES pair of hasDirectoryFlow). */ + subscribeDirectoryFlow: (listener: () => void) => () => void /** Render this surface's directory-flow hole with the owner conversation (the entry's narrowed renderSlot). */ renderDirectoryFlow: (owner: DirectoryFlowOwnerProps) => ReactNode /** A real Workspace was picked or created. */ @@ -60,6 +62,7 @@ export function WorkspaceCreateFlow({ useWorkspaces, createWorkspace, hasDirectoryFlow, + subscribeDirectoryFlow, renderDirectoryFlow, onPick, onClose, @@ -91,11 +94,17 @@ export function WorkspaceCreateFlow({ && workspaces.some(workspace => workspace.title === normalizedWorkspaceName) // The occupied hole gates the picking affordance: with no composed flow the - // entry simply is not there (the seam's documented no-flow default). Read - // per render while the menu is open — registrations land through plugin - // activation, and the menu re-renders on every toggle. + // entry simply is not there (the seam's documented no-flow default). The + // subscription keeps occupancy live: flow plugins activate (and HMR-reload) + // independently of this menu's renders. + const flowAvailable = useSyncExternalStore(subscribeDirectoryFlow, hasDirectoryFlow) + // An occupant that unloads mid-interaction leaves nobody to cancel: an + // open flow over an empty hole withdraws so the menu actions come back. + useEffect(() => { + if (!flowAvailable) setFlowOpen(false) + }, [flowAvailable]) const createEntries: MenuEntry[] = [ - ...(hasDirectoryFlow() + ...(flowAvailable ? [{ id: OPEN_LOCAL_FOLDER, label: 'Open local folder…', icon: , disabled: flowBusy }] : []), { id: CREATE_NEW, label: 'Create a new workspace', icon: , disabled: flowBusy }, @@ -288,6 +297,7 @@ export function WorkspacePicker({ onClose, createWorkspace, hasDirectoryFlow, + subscribeDirectoryFlow, renderSlot, }: WorkspacePickerProps) { return ( @@ -297,6 +307,7 @@ export function WorkspacePicker({ useWorkspaces={useWorkspaces} createWorkspace={createWorkspace} hasDirectoryFlow={hasDirectoryFlow} + subscribeDirectoryFlow={subscribeDirectoryFlow} renderDirectoryFlow={owner => renderSlot('conversation.hero.workspace.directoryFlow', owner)} selectedId={selectedId} onPick={onPick} diff --git a/packages/client/ui-workspace/src/client/contract/slots.ts b/packages/client/ui-workspace/src/client/contract/slots.ts index fce3bd9b46..34c59d764f 100644 --- a/packages/client/ui-workspace/src/client/contract/slots.ts +++ b/packages/client/ui-workspace/src/client/contract/slots.ts @@ -62,11 +62,18 @@ export type DirectoryFlowSlotName = /** Directory-picking share both trigger surfaces consume. */ export type DirectoryPickingInjected = { /** - * Whether this surface's directory-flow hole is occupied — read when the - * menu opens; an empty hole hides the "Open local folder…" entry (the - * no-flow composition simply has no picking affordance). + * Whether this surface's directory-flow hole is occupied — an empty hole + * hides the "Open local folder…" entry (the no-flow composition simply has + * no picking affordance). */ hasDirectoryFlow: () => boolean + /** + * Subscribe to the hole's registration changes (the uSES pair of + * {@link hasDirectoryFlow}): the trigger surface withdraws an open flow + * whose occupant unloaded mid-interaction — nobody is left to cancel it. + * @returns the unsubscriber. + */ + subscribeDirectoryFlow: (listener: () => void) => () => void } /** diff --git a/packages/client/ui-workspace/src/client/index.ts b/packages/client/ui-workspace/src/client/index.ts index 4fd9edb38c..9dbe2ed2e6 100644 --- a/packages/client/ui-workspace/src/client/index.ts +++ b/packages/client/ui-workspace/src/client/index.ts @@ -49,10 +49,12 @@ export function apply(ctx: ClientContext): void { }, createWorkspace: input => ctx.workspaces.create(input), hasDirectoryFlow: () => ctx.slots.entries('sidebar.workspaces.directoryFlow').length > 0, + subscribeDirectoryFlow: listener => ctx.slots.subscribe('sidebar.workspaces.directoryFlow', listener), }) const pickerInjected = (): WorkspacePickerInjected => ({ createWorkspace: input => ctx.workspaces.create(input), hasDirectoryFlow: () => ctx.slots.entries('conversation.hero.workspace.directoryFlow').length > 0, + subscribeDirectoryFlow: listener => ctx.slots.subscribe('conversation.hero.workspace.directoryFlow', listener), }) // Declaration-aware registration (deferRegistration): each owner's // declaring apply may activate after this one, and a register into an diff --git a/packages/client/ui-workspace/tests/apply.spec.ts b/packages/client/ui-workspace/tests/apply.spec.ts index 4ef3738d01..7d5dac71de 100644 --- a/packages/client/ui-workspace/tests/apply.spec.ts +++ b/packages/client/ui-workspace/tests/apply.spec.ts @@ -91,11 +91,16 @@ describe('ui-workspace apply', () => { expect(browser.hasDirectoryFlow()).toBe(false) expect(picker.hasDirectoryFlow()).toBe(false) // A flow occupant flips exactly its own surface. + const notified = vi.fn() + const unsubscribe = browser.subscribeDirectoryFlow(notified) const dispose = b.slots.register({ name: 'sidebar.workspaces.directoryFlow' } as never, () => null) expect(browser.hasDirectoryFlow()).toBe(true) expect(picker.hasDirectoryFlow()).toBe(false) + await Promise.resolve() + expect(notified).toHaveBeenCalled() dispose() expect(browser.hasDirectoryFlow()).toBe(false) + unsubscribe() }) it('unregisters every entry on teardown', async () => { diff --git a/packages/client/ui-workspace/tests/workspace-browser.spec.tsx b/packages/client/ui-workspace/tests/workspace-browser.spec.tsx index e4b038b0f3..f1d43997f7 100644 --- a/packages/client/ui-workspace/tests/workspace-browser.spec.tsx +++ b/packages/client/ui-workspace/tests/workspace-browser.spec.tsx @@ -60,6 +60,7 @@ function mount(overrides: Partial = {}) { insertSessionBefore: vi.fn(async () => {}), createWorkspace: vi.fn(async () => workspace('created', [])), hasDirectoryFlow: () => true, + subscribeDirectoryFlow: () => () => {}, renderSlot: ((_name: string, owner: { open: boolean }) => (owner.open ?
: null)) as never, ...overrides, } diff --git a/packages/client/ui-workspace/tests/workspace-picker.spec.tsx b/packages/client/ui-workspace/tests/workspace-picker.spec.tsx index 61d353fe45..1f81ac59bf 100644 --- a/packages/client/ui-workspace/tests/workspace-picker.spec.tsx +++ b/packages/client/ui-workspace/tests/workspace-picker.spec.tsx @@ -50,10 +50,27 @@ function flowProbe() { return { probe, renderSlot } } +/** Manual occupancy source: flip() drives the uSES subscription like a real registration change. */ +function occupancySource(initial = true) { + let occupied = initial + const listeners = new Set<() => void>() + return { + hasDirectoryFlow: () => occupied, + subscribeDirectoryFlow: (listener: () => void) => { + listeners.add(listener) + return () => { listeners.delete(listener) } + }, + flip: (next: boolean) => { + occupied = next + for (const listener of [...listeners]) listener() + }, + } +} + function mount( items: readonly WorkspaceView[] = [workspace('alpha', 'Alpha')], createWorkspace = vi.fn(), - hasDirectoryFlow: () => boolean = () => true, + occupancy = occupancySource(), ) { const onPick = vi.fn() const onClose = vi.fn() @@ -68,7 +85,8 @@ function mount( onPick={onPick} onClose={onClose} createWorkspace={createWorkspace} - hasDirectoryFlow={hasDirectoryFlow} + hasDirectoryFlow={occupancy.hasDirectoryFlow} + subscribeDirectoryFlow={occupancy.subscribeDirectoryFlow} renderSlot={renderSlot} /> ) @@ -76,7 +94,7 @@ function mount( renderPicker(items), ) return { - view, onPick, onClose, createWorkspace, probe, + view, onPick, onClose, createWorkspace, probe, occupancy, rerenderItems: (nextItems: readonly WorkspaceView[]) => { view.rerender(renderPicker(nextItems)) }, } } @@ -247,7 +265,7 @@ describe('WorkspacePicker', () => { true} renderSlot={renderSlot} + hasDirectoryFlow={() => true} subscribeDirectoryFlow={() => () => {}} renderSlot={renderSlot} />, ) expect(screen.queryByRole('menu')).toBeNull() @@ -262,26 +280,35 @@ describe('WorkspacePicker', () => { true} renderSlot={renderSlot} + hasDirectoryFlow={() => true} subscribeDirectoryFlow={() => () => {}} renderSlot={renderSlot} />, ) expect(screen.getByRole('status').textContent).toBe('Loading workspaces…') }) it('hides the folder entry while the directory-flow hole is empty', () => { - mount([], vi.fn(), () => false) + mount([], vi.fn(), occupancySource(false)) expect(screen.getByRole('menuitem', { name: 'Create a new workspace' })).toBeTruthy() expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull() }) - it('shows the folder entry once the hole reports an occupant on a later render', () => { - let occupied = false - const b = mount([], vi.fn(), () => occupied) + it('shows the folder entry when a flow package activates after the first paint', () => { + const b = mount([], vi.fn(), occupancySource(false)) expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull() - // A flow package activating after the first paint is observed on the - // next render — the same cadence as reopening the menu. - occupied = true - b.rerenderItems([]) + // Registration changes flow through the subscription, no re-render needed. + act(() => { b.occupancy.flip(true) }) expect(screen.getByRole('menuitem', { name: 'Open local folder…' })).toBeTruthy() }) + + it('withdraws an open flow when its occupant unloads, re-enabling the menu actions', () => { + const b = mount([]) + chooseItem('Open local folder…') + expect(screen.getByTestId('directory-flow')).toBeTruthy() + // The flow plugin unloads mid-interaction (HMR): nobody is left to + // cancel, so the owner withdraws and the actions come back. + act(() => { b.occupancy.flip(false) }) + expect(b.probe.owner!.open).toBe(false) + expect(screen.getByRole('menuitem', { name: 'Create a new workspace' }).disabled).toBe(false) + expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull() + }) })