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).
This commit is contained in:
creatixchu
2026-07-29 02:36:08 +08:00
parent 3f23347d10
commit 7114cc7475
7 changed files with 77 additions and 22 deletions

View File

@@ -254,6 +254,7 @@ export function WorkspaceBrowser({
insertSessionBefore, insertSessionBefore,
createWorkspace, createWorkspace,
hasDirectoryFlow, hasDirectoryFlow,
subscribeDirectoryFlow,
renderSlot, renderSlot,
}: WorkspaceBrowserProps) { }: WorkspaceBrowserProps) {
const workspaces = useWorkspaces(state => state.items) const workspaces = useWorkspaces(state => state.items)
@@ -373,6 +374,7 @@ export function WorkspaceBrowser({
useWorkspaces={useWorkspaces} useWorkspaces={useWorkspaces}
createWorkspace={createWorkspace} createWorkspace={createWorkspace}
hasDirectoryFlow={hasDirectoryFlow} hasDirectoryFlow={hasDirectoryFlow}
subscribeDirectoryFlow={subscribeDirectoryFlow}
renderDirectoryFlow={owner => renderSlot('sidebar.workspaces.directoryFlow', owner)} renderDirectoryFlow={owner => renderSlot('sidebar.workspaces.directoryFlow', owner)}
createOnly createOnly
side="right" side="right"

View File

@@ -7,7 +7,7 @@
* opens the flow, adopts the picked path, and owns the error surface. * opens the flow, adopts the picked path, and owns the error surface.
*/ */
import type { ReactNode, RefObject } from 'react' import type { ReactNode, RefObject } from 'react'
import { useCallback, useRef, useState } from 'react' import { useCallback, useEffect, useRef, useState, useSyncExternalStore } from 'react'
import { import {
Button, IconFolderClose16, IconPlusOutline16, Menu, Modal, type MenuEntry, Button, IconFolderClose16, IconPlusOutline16, Menu, Modal, type MenuEntry,
} from '@deepseek-ai/dsh-client-ui-primitives' } from '@deepseek-ai/dsh-client-ui-primitives'
@@ -33,8 +33,10 @@ export interface WorkspaceCreateFlowProps {
useWorkspaces: <S>(selector: (state: WorkspaceListState) => S) => S useWorkspaces: <S>(selector: (state: WorkspaceListState) => S) => S
/** Create or adopt a real Host Workspace. */ /** Create or adopt a real Host Workspace. */
createWorkspace: (input: { name: string } | { path: string }) => Promise<WorkspaceView> createWorkspace: (input: { name: string } | { path: string }) => Promise<WorkspaceView>
/** 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 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). */ /** Render this surface's directory-flow hole with the owner conversation (the entry's narrowed renderSlot). */
renderDirectoryFlow: (owner: DirectoryFlowOwnerProps) => ReactNode renderDirectoryFlow: (owner: DirectoryFlowOwnerProps) => ReactNode
/** A real Workspace was picked or created. */ /** A real Workspace was picked or created. */
@@ -60,6 +62,7 @@ export function WorkspaceCreateFlow({
useWorkspaces, useWorkspaces,
createWorkspace, createWorkspace,
hasDirectoryFlow, hasDirectoryFlow,
subscribeDirectoryFlow,
renderDirectoryFlow, renderDirectoryFlow,
onPick, onPick,
onClose, onClose,
@@ -91,11 +94,17 @@ export function WorkspaceCreateFlow({
&& workspaces.some(workspace => workspace.title === normalizedWorkspaceName) && workspaces.some(workspace => workspace.title === normalizedWorkspaceName)
// The occupied hole gates the picking affordance: with no composed flow the // 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 // entry simply is not there (the seam's documented no-flow default). The
// per render while the menu is open — registrations land through plugin // subscription keeps occupancy live: flow plugins activate (and HMR-reload)
// activation, and the menu re-renders on every toggle. // 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[] = [ const createEntries: MenuEntry[] = [
...(hasDirectoryFlow() ...(flowAvailable
? [{ id: OPEN_LOCAL_FOLDER, label: 'Open local folder…', icon: <IconFolderClose16 size={16} />, disabled: flowBusy }] ? [{ id: OPEN_LOCAL_FOLDER, label: 'Open local folder…', icon: <IconFolderClose16 size={16} />, disabled: flowBusy }]
: []), : []),
{ id: CREATE_NEW, label: 'Create a new workspace', icon: <IconPlusOutline16 size={16} />, disabled: flowBusy }, { id: CREATE_NEW, label: 'Create a new workspace', icon: <IconPlusOutline16 size={16} />, disabled: flowBusy },
@@ -288,6 +297,7 @@ export function WorkspacePicker({
onClose, onClose,
createWorkspace, createWorkspace,
hasDirectoryFlow, hasDirectoryFlow,
subscribeDirectoryFlow,
renderSlot, renderSlot,
}: WorkspacePickerProps) { }: WorkspacePickerProps) {
return ( return (
@@ -297,6 +307,7 @@ export function WorkspacePicker({
useWorkspaces={useWorkspaces} useWorkspaces={useWorkspaces}
createWorkspace={createWorkspace} createWorkspace={createWorkspace}
hasDirectoryFlow={hasDirectoryFlow} hasDirectoryFlow={hasDirectoryFlow}
subscribeDirectoryFlow={subscribeDirectoryFlow}
renderDirectoryFlow={owner => renderSlot('conversation.hero.workspace.directoryFlow', owner)} renderDirectoryFlow={owner => renderSlot('conversation.hero.workspace.directoryFlow', owner)}
selectedId={selectedId} selectedId={selectedId}
onPick={onPick} onPick={onPick}

View File

@@ -62,11 +62,18 @@ export type DirectoryFlowSlotName =
/** Directory-picking share both trigger surfaces consume. */ /** Directory-picking share both trigger surfaces consume. */
export type DirectoryPickingInjected = { export type DirectoryPickingInjected = {
/** /**
* Whether this surface's directory-flow hole is occupied — read when the * Whether this surface's directory-flow hole is occupied — an empty hole
* menu opens; an empty hole hides the "Open local folder…" entry (the * hides the "Open local folder…" entry (the no-flow composition simply has
* no-flow composition simply has no picking affordance). * no picking affordance).
*/ */
hasDirectoryFlow: () => boolean 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
} }
/** /**

View File

@@ -49,10 +49,12 @@ export function apply(ctx: ClientContext): void {
}, },
createWorkspace: input => ctx.workspaces.create(input), createWorkspace: input => ctx.workspaces.create(input),
hasDirectoryFlow: () => ctx.slots.entries('sidebar.workspaces.directoryFlow').length > 0, hasDirectoryFlow: () => ctx.slots.entries('sidebar.workspaces.directoryFlow').length > 0,
subscribeDirectoryFlow: listener => ctx.slots.subscribe('sidebar.workspaces.directoryFlow', listener),
}) })
const pickerInjected = (): WorkspacePickerInjected => ({ const pickerInjected = (): WorkspacePickerInjected => ({
createWorkspace: input => ctx.workspaces.create(input), createWorkspace: input => ctx.workspaces.create(input),
hasDirectoryFlow: () => ctx.slots.entries('conversation.hero.workspace.directoryFlow').length > 0, 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 // Declaration-aware registration (deferRegistration): each owner's
// declaring apply may activate after this one, and a register into an // declaring apply may activate after this one, and a register into an

View File

@@ -91,11 +91,16 @@ describe('ui-workspace apply', () => {
expect(browser.hasDirectoryFlow()).toBe(false) expect(browser.hasDirectoryFlow()).toBe(false)
expect(picker.hasDirectoryFlow()).toBe(false) expect(picker.hasDirectoryFlow()).toBe(false)
// A flow occupant flips exactly its own surface. // 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) const dispose = b.slots.register({ name: 'sidebar.workspaces.directoryFlow' } as never, () => null)
expect(browser.hasDirectoryFlow()).toBe(true) expect(browser.hasDirectoryFlow()).toBe(true)
expect(picker.hasDirectoryFlow()).toBe(false) expect(picker.hasDirectoryFlow()).toBe(false)
await Promise.resolve()
expect(notified).toHaveBeenCalled()
dispose() dispose()
expect(browser.hasDirectoryFlow()).toBe(false) expect(browser.hasDirectoryFlow()).toBe(false)
unsubscribe()
}) })
it('unregisters every entry on teardown', async () => { it('unregisters every entry on teardown', async () => {

View File

@@ -60,6 +60,7 @@ function mount(overrides: Partial<WorkspaceBrowserProps> = {}) {
insertSessionBefore: vi.fn(async () => {}), insertSessionBefore: vi.fn(async () => {}),
createWorkspace: vi.fn(async () => workspace('created', [])), createWorkspace: vi.fn(async () => workspace('created', [])),
hasDirectoryFlow: () => true, hasDirectoryFlow: () => true,
subscribeDirectoryFlow: () => () => {},
renderSlot: ((_name: string, owner: { open: boolean }) => (owner.open ? <div data-testid="directory-flow" /> : null)) as never, renderSlot: ((_name: string, owner: { open: boolean }) => (owner.open ? <div data-testid="directory-flow" /> : null)) as never,
...overrides, ...overrides,
} }

View File

@@ -50,10 +50,27 @@ function flowProbe() {
return { probe, renderSlot } 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( function mount(
items: readonly WorkspaceView[] = [workspace('alpha', 'Alpha')], items: readonly WorkspaceView[] = [workspace('alpha', 'Alpha')],
createWorkspace = vi.fn(), createWorkspace = vi.fn(),
hasDirectoryFlow: () => boolean = () => true, occupancy = occupancySource(),
) { ) {
const onPick = vi.fn() const onPick = vi.fn()
const onClose = vi.fn() const onClose = vi.fn()
@@ -68,7 +85,8 @@ function mount(
onPick={onPick} onPick={onPick}
onClose={onClose} onClose={onClose}
createWorkspace={createWorkspace} createWorkspace={createWorkspace}
hasDirectoryFlow={hasDirectoryFlow} hasDirectoryFlow={occupancy.hasDirectoryFlow}
subscribeDirectoryFlow={occupancy.subscribeDirectoryFlow}
renderSlot={renderSlot} renderSlot={renderSlot}
/> />
) )
@@ -76,7 +94,7 @@ function mount(
renderPicker(items), renderPicker(items),
) )
return { return {
view, onPick, onClose, createWorkspace, probe, view, onPick, onClose, createWorkspace, probe, occupancy,
rerenderItems: (nextItems: readonly WorkspaceView[]) => { view.rerender(renderPicker(nextItems)) }, rerenderItems: (nextItems: readonly WorkspaceView[]) => { view.rerender(renderPicker(nextItems)) },
} }
} }
@@ -247,7 +265,7 @@ describe('WorkspacePicker', () => {
<WorkspacePicker <WorkspacePicker
open useSessions={hook(sessions)} useWorkspaces={hook(workspaceState([]))} open useSessions={hook(sessions)} useWorkspaces={hook(workspaceState([]))}
onPick={vi.fn()} onClose={vi.fn()} createWorkspace={vi.fn()} onPick={vi.fn()} onClose={vi.fn()} createWorkspace={vi.fn()}
hasDirectoryFlow={() => true} renderSlot={renderSlot} hasDirectoryFlow={() => true} subscribeDirectoryFlow={() => () => {}} renderSlot={renderSlot}
/>, />,
) )
expect(screen.queryByRole('menu')).toBeNull() expect(screen.queryByRole('menu')).toBeNull()
@@ -262,26 +280,35 @@ describe('WorkspacePicker', () => {
<WorkspacePicker <WorkspacePicker
open anchorRef={anchor()} useSessions={hook(sessions)} useWorkspaces={hook(state)} open anchorRef={anchor()} useSessions={hook(sessions)} useWorkspaces={hook(state)}
onPick={vi.fn()} onClose={vi.fn()} createWorkspace={vi.fn()} onPick={vi.fn()} onClose={vi.fn()} createWorkspace={vi.fn()}
hasDirectoryFlow={() => true} renderSlot={renderSlot} hasDirectoryFlow={() => true} subscribeDirectoryFlow={() => () => {}} renderSlot={renderSlot}
/>, />,
) )
expect(screen.getByRole('status').textContent).toBe('Loading workspaces…') expect(screen.getByRole('status').textContent).toBe('Loading workspaces…')
}) })
it('hides the folder entry while the directory-flow hole is empty', () => { 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.getByRole('menuitem', { name: 'Create a new workspace' })).toBeTruthy()
expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull() expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull()
}) })
it('shows the folder entry once the hole reports an occupant on a later render', () => { it('shows the folder entry when a flow package activates after the first paint', () => {
let occupied = false const b = mount([], vi.fn(), occupancySource(false))
const b = mount([], vi.fn(), () => occupied)
expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull() expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull()
// A flow package activating after the first paint is observed on the // Registration changes flow through the subscription, no re-render needed.
// next render — the same cadence as reopening the menu. act(() => { b.occupancy.flip(true) })
occupied = true
b.rerenderItems([])
expect(screen.getByRole('menuitem', { name: 'Open local folder…' })).toBeTruthy() 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<HTMLButtonElement>('menuitem', { name: 'Create a new workspace' }).disabled).toBe(false)
expect(screen.queryByRole('menuitem', { name: 'Open local folder…' })).toBeNull()
})
}) })