From 6add9a8eef7ea8937d4a2253be8ac346a3a0d490 Mon Sep 17 00:00:00 2001
From: creatixchu <creatixchu@deepseek.com>
Date: Tue, 28 Jul 2026 23:37:07 +0800
Subject: [PATCH] fix(host): harden the browser dialog against review round-2
 races
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Dismissal (Escape/mask) is ignored while adoption is busy: the owner's
  in-flight createWorkspace must not land behind an apparent cancel.
- Every parent control goes inert while the nested create dialog is open
  (Modal traps no focus, so Shift-Tab/AT could close, adopt, or retarget
  underneath the child).
- Creation settlements are gated on an open-generation ref: a create that
  resolves or rejects after the flow closed (and possibly reopened) can no
  longer relist the stale target or surface its alert in the fresh dialog.
- The keyless snapshot waits for the Open button's enabled state before
  clicking — on slow runners the selection's child listing was still in
  flight and the click landed on a disabled button.
---
 apps/web/tests/workspace-flow.snapshot.ts     |  5 ++
 .../src/client/DirectoryBrowser.tsx           | 36 +++++++----
 .../tests/directory-browser.spec.tsx          | 59 +++++++++++++++++++
 3 files changed, 90 insertions(+), 10 deletions(-)

diff --git a/apps/web/tests/workspace-flow.snapshot.ts b/apps/web/tests/workspace-flow.snapshot.ts
index 8d3316ab0a..86a20e73fe 100644
--- a/apps/web/tests/workspace-flow.snapshot.ts
+++ b/apps/web/tests/workspace-flow.snapshot.ts
@@ -202,6 +202,11 @@ it('adopts a directory through the composed in-app browse flow and lands in its
   // row's name span is stable (clicks bubble to the row button).
   fireEvent.click(await within(dialog).findByText('Documents', {}, { timeout: 10_000 }))
   fireEvent.click(await within(dialog).findByText('project', {}, { timeout: 10_000 }))
+  // Open disables while the selection's child listing is in flight; wait for
+  // the enabled state or the click lands on a dead button on slow runners.
+  await waitFor(() => {
+    expect(within(dialog).getByRole<HTMLButtonElement>('button', { name: '打开' }).disabled).toBe(false)
+  }, { timeout: 10_000 })
   fireEvent.click(within(dialog).getByRole('button', { name: '打开' }))
   await findHeroComposer()
   await waitFor(() => {
diff --git a/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx b/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx
index a3bbc576a7..ec66c6e994 100644
--- a/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx
+++ b/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx
@@ -112,6 +112,9 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
   const [creatingFolder, setCreatingFolder] = useState(false)
   const [createError, setCreateError] = useState<string | null>(null)
   const requestSeq = useRef(0)
+  // Bumped on every open/close edge: settlements from a previous open (a
+  // pending creation included) must never mutate a reopened dialog.
+  const openGeneration = useRef(0)
   // Deep ancestry overflows the trail; keep its tail (the current directory
   // and the edit zone beside it) in view whenever the chain changes.
   const crumbTrailRef = useRef<HTMLSpanElement | null>(null)
@@ -164,10 +167,12 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
   // Every open starts fresh at the Host home directory; closing invalidates
   // any in-flight response so a late arrival cannot repopulate a closed dialog.
   useEffect(() => {
+    openGeneration.current += 1
     if (open) {
       setParent(null)
       setSelected(null)
       setChild(null)
+      setCreatingFolder(false)
       navigate()
       return
     }
@@ -190,7 +195,11 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
     if (name === '') return
     setCreatingFolder(true)
     setCreateError(null)
+    const generation = openGeneration.current
     createDirectory(targetPath, name).then((createdPath) => {
+      // A settlement from a closed (possibly reopened) flow must not touch
+      // the fresh dialog or issue a relist against the stale target.
+      if (generation !== openGeneration.current) return
       setCreatingFolder(false)
       setFolderDraft(null)
       // Land like a right-column pick (figma 802:57446 → 813:23278 flow): the
@@ -210,6 +219,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
         setError(failureText(reason))
       })
     }, (reason: unknown) => {
+      if (generation !== openGeneration.current) return
       setCreatingFolder(false)
       setCreateError(failureText(reason))
     })
@@ -226,14 +236,20 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
 
   if (!open) return null
   const twoPane = selected !== null
+  // The nested create dialog owns the interaction while open: Modal has no
+  // focus trap, so every parent control goes inert (Shift-Tab or AT must not
+  // close, adopt, or retarget underneath the child).
+  const parentInert = busy || folderDraft !== null
 
   return (
     <Modal
       open={open}
       // Escape and mask reach every mounted Modal's document listener; while
-      // the nested create dialog is up, only that topmost dialog may close
-      // (its own guard keeps an in-flight creation open).
-      onClose={() => { if (folderDraft === null) onClose() }}
+      // the nested create dialog is up only that topmost dialog may close
+      // (its own guard keeps an in-flight creation open), and an in-flight
+      // adoption pins the flow — dismissing it would leave the owner's
+      // createWorkspace to land after an apparent cancel.
+      onClose={() => { if (folderDraft === null && !busy) onClose() }}
       title={t('browser.title')}
       className={clsx(css.dialog)}
       headless
@@ -251,7 +267,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
                       <button
                         type="button"
                         className={css.crumb}
-                        disabled={busy}
+                        disabled={parentInert}
                         onClick={() => { navigate(crumb.path) }}
                       >
                         {crumb.name}
@@ -264,7 +280,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
                   type="button"
                   className={css.crumbEditZone}
                   aria-label={t('browser.editPath')}
-                  disabled={parent === null || busy}
+                  disabled={parent === null || parentInert}
                   /* v8 ignore next -- narrowing guard: the zone disables while the level is null. */
                   onClick={() => { if (parent !== null) setPathDraft(selected?.path ?? parent.path) }}
                 />
@@ -299,7 +315,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
           <LevelColumn
             entries={parent.entries}
             selectedPath={selected?.path ?? null}
-            busy={busy}
+            busy={parentInert}
             onPick={select}
             wide={!twoPane}
           />
@@ -309,7 +325,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
           <LevelColumn
             entries={child.entries}
             selectedPath={null}
-            busy={busy}
+            busy={parentInert}
             onPick={advance}
             wide={false}
           />
@@ -321,7 +337,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
         <Button
           variant="outline"
           icon={<IconPlusOutline16 size={14} />}
-          disabled={parent === null || busy || loading || folderDraft !== null}
+          disabled={parent === null || loading || parentInert}
           onClick={() => {
             setFolderDraft('')
             setCreateError(null)
@@ -330,11 +346,11 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
           {t('browser.newFolder')}
         </Button>
         <span className={css.footerGap} />
-        <Button variant="outline" className={clsx(css.footerAction)} disabled={busy} onClick={onClose}>{t('browser.cancel')}</Button>
+        <Button variant="outline" className={clsx(css.footerAction)} disabled={parentInert} onClick={onClose}>{t('browser.cancel')}</Button>
         <Button
           variant="primary"
           className={clsx(css.footerAction)}
-          disabled={targetPath === null || loading || busy}
+          disabled={targetPath === null || loading || parentInert}
           /* v8 ignore next -- narrowing guard: Open disables while no target exists. */
           onClick={() => { if (targetPath !== null) onOpen(targetPath) }}
         >
diff --git a/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx b/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx
index 2c513b9397..5b656496d8 100644
--- a/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx
+++ b/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx
@@ -264,6 +264,65 @@ describe('DirectoryBrowser', () => {
     expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.newFolder' }).disabled).toBe(false)
   })
 
+  it('ignores dismissal while adoption is busy', async () => {
+    const b = mount({ busy: true })
+    await waitFor(() => { expect(screen.getByRole('dialog')).toBeTruthy() })
+    fireEvent.keyDown(document, { key: 'Escape' })
+    expect(b.onClose).not.toHaveBeenCalled()
+  })
+
+  it('makes every parent control inert while the nested create dialog is open', async () => {
+    mount()
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
+    // Modal traps no focus: Shift-Tab/AT reach the parent, so closing,
+    // adopting, and retargeting must all disable underneath the child. Both
+    // dialogs carry a cancel: the parent's disables, the child's stays live.
+    const cancels = screen.getAllByRole<HTMLButtonElement>('button', { name: 'browser.cancel' })
+    expect(cancels.map(button => button.disabled).sort()).toEqual([false, true])
+    expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.open' }).disabled).toBe(true)
+    expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.editPath' }).disabled).toBe(true)
+    for (const row of screen.getAllByRole<HTMLButtonElement>('listitem')) {
+      expect(row.disabled).toBe(true)
+    }
+  })
+
+  it('drops a creation failure that lands after the flow closed and reopened', async () => {
+    let rejectCreate!: (reason: unknown) => void
+    const createDirectory = vi.fn(() => new Promise<string>((_settle, reject) => { rejectCreate = reject }))
+    const b = mount({ createDirectory })
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
+    fireEvent.change(screen.getByLabelText('browser.folderName'), { target: { value: 'slow' } })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.create' }))
+    b.view.rerender(<DirectoryBrowser {...b.props} open={false} />)
+    b.view.rerender(<DirectoryBrowser {...b.props} open />)
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    // The stale failure must not surface an alert inside the fresh flow.
+    await act(async () => { rejectCreate(new Error('too late')) })
+    expect(screen.queryByText('too late')).toBeNull()
+  })
+
+  it('drops a creation that settles after the flow closed and reopened', async () => {
+    let settleCreate!: (path: string) => void
+    const createDirectory = vi.fn(() => new Promise<string>((settle) => { settleCreate = settle }))
+    const b = mount({ createDirectory })
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
+    fireEvent.change(screen.getByLabelText('browser.folderName'), { target: { value: 'slow' } })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.create' }))
+    b.view.rerender(<DirectoryBrowser {...b.props} open={false} />)
+    b.view.rerender(<DirectoryBrowser {...b.props} open />)
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    const listCallsBefore = b.listDirectory.mock.calls.length
+    // The stale settlement must not relist the old target or reopen the
+    // nested dialog's state inside the fresh flow.
+    await act(async () => { settleCreate(`${HOME}/slow`) })
+    expect(b.listDirectory.mock.calls.length).toBe(listCallsBefore)
+    expect(screen.queryByLabelText('browser.folderName')).toBeNull()
+    expect(screen.getByText('Documents')).toBeTruthy()
+  })
+
   it('creates a folder through the nested dialog and lands with it selected', async () => {
     const b = mount()
     await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })