fix(host): address review — Escape scoping, relist gating, crumb overflow, aria golden

- Escape (and the mask) now reaches only the topmost dialog: while the nested New-folder dialog is up the browser ignores its own Modal close, and the nested dialog's in-flight fence keeps both open during creation. - New folder disables while any listing loads, so a slow post-create relist/select sequence cannot host a second create against a target the pending listing is about to change. - Deep ancestry scrolls inside a dedicated crumb trail whose tail is pinned into view; the path-edit zone keeps its reserved width instead of being clipped by the card, preserving cross-drive path entry. - The workspace-management e2e records a directory-browser aria golden at a staged tree (host HOME pointed at the scaffold cwd collapses ancestry into the Home crumb, keeping the artifact machine-independent), and the keyless snapshot's row targeting goes through visible label text — listitem accessible-name computation differs across dom-accessibility-api environments (the CI-only miss).
2026-08-15 21:04:50 +00:00 · 2026-07-28 23:21:27 +08:00
parent 0d9ac53fb0
commit 6cd63f741c
6 changed files with 161 additions and 26 deletions
--- a/apps/web/tests/snapshots/workspace-management/directory-browser.expected.md
+++ b/apps/web/tests/snapshots/workspace-management/directory-browser.expected.md
@@ -0,0 +1,20 @@
+- dialog "选择工作区目录":
+  - heading "选择工作区目录" [level=2]
+  - button "主目录"
+  - img
+  - button "browse-golden"
+  - button "编辑路径"
+  - list:
+    - listitem:
+      - img
+      - text: alpha
+      - img
+    - listitem:
+      - img
+      - text: beta
+      - img
+  - button "新建文件夹":
+    - img
+    - text: 新建文件夹
+  - button "取消"
+  - button "打开"
--- a/apps/web/tests/workspace-flow.snapshot.ts
+++ b/apps/web/tests/workspace-flow.snapshot.ts
@@ -197,8 +197,11 @@ it('adopts a directory through the composed in-app browse flow and lands in its
  // The browse occupant renders the Select Workspace Directory dialog at the
  // fixture home; select Documents, advance into project, and adopt it.
  const dialog = await screen.findByRole('dialog', { name: '选择工作区目录' }, { timeout: 10_000 })
-  fireEvent.click(await within(dialog).findByRole('listitem', { name: /Documents/ }, { timeout: 10_000 }))
-  fireEvent.click(await within(dialog).findByRole('listitem', { name: /^project/ }, { timeout: 10_000 }))
+  // Row targeting goes through the visible label text: listitem accessible-name
+  // computation differs across dom-accessibility-api environments, while the
+  // 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 }))
  fireEvent.click(within(dialog).getByRole('button', { name: '打开' }))
  await findHeroComposer()
  await waitFor(() => {
--- a/apps/web/tests/workspace-management.e2e.ts
+++ b/apps/web/tests/workspace-management.e2e.ts
@@ -13,8 +13,8 @@ import { chromium } from 'playwright'
 import { afterAll, beforeAll, describe, expect, it, onTestFailed } from 'vitest'
 import { SessionId } from '@deepseek-ai/dsh-session'
 import {
-  acknowledgeReloadConnectionLoss, assertFixtureInventory, launchWebScaffold, seedSession, watchConsole,
-  webSnapshotMode, type WebScaffold,
+  acknowledgeReloadConnectionLoss, assertFixtureInventory, captureStableAria, compareOrRefreshGolden,
+  launchWebScaffold, seedSession, watchConsole, webSnapshotMode, type WebScaffold,
 } from './scaffold.ts'
 import { saveFailureShot } from './support.ts'

@@ -23,6 +23,7 @@ const SNAPSHOT_DIR = fileURLToPath(new URL('./snapshots/workspace-management', i
 // spec needs any one cold session row, not new recorded content.
 const SEED = fileURLToPath(new URL('./snapshots/seeded-history/seed.jsonl', import.meta.url))
 const MODE = webSnapshotMode()
+const BROWSER_EXPECTED = join(SNAPSHOT_DIR, 'directory-browser.expected.md')
 const SEED_ID = 'workspace-management-web-e2e'

 describe('web e2e: workspace management (create / rename / flat view / hover card)', () => {
@@ -345,6 +346,35 @@ describe('web e2e: workspace management (create / rename / flat view / hover car
    expect(tripwire.pageErrors).toEqual([])
  }, 90_000)

+  it('matches the directory-browser dialog aria golden at a staged directory', async () => {
+    // A staged subtree under the scaffold cwd keeps the listing deterministic
+    // (normalizeAria scrubs the cwd), and pointing the in-process host's HOME
+    // at the cwd collapses the breadcrumb ancestry into the Home crumb — no
+    // machine-specific path segments or real $HOME contents enter the golden.
+    const staged = join(scaffold.workspaceCwd, 'browse-golden')
+    await mkdir(join(staged, 'alpha'), { recursive: true })
+    await mkdir(join(staged, 'beta'), { recursive: true })
+    const realHome = process.env.HOME
+    process.env.HOME = scaffold.workspaceCwd
+    try {
+      await page.getByRole('button', { name: 'Create workspace' }).click()
+      await page.getByRole('menuitem', { name: 'Open local folder…' }).click()
+      const dialog = page.getByRole('dialog', { name: '选择工作区目录' })
+      await dialog.waitFor({ timeout: 10_000 })
+      await dialog.getByRole('button', { name: '编辑路径' }).click()
+      await dialog.getByLabel('编辑路径').fill(staged)
+      await dialog.getByLabel('编辑路径').press('Enter')
+      await expect.poll(() => dialog.getByText('alpha', { exact: true }).count(), { timeout: 10_000 }).toBe(1)
+      const snapshot = await captureStableAria(page, '[role="dialog"]', scaffold.workspaceCwd)
+      await compareOrRefreshGolden(BROWSER_EXPECTED, snapshot, MODE)
+      await dialog.getByRole('button', { name: '取消' }).click()
+      await dialog.waitFor({ state: 'hidden', timeout: 10_000 })
+    } finally {
+      process.env.HOME = realHome
+    }
+    expect(tripwire.pageErrors).toEqual([])
+  }, 60_000)
+
  it('shows the session hover card after a dwell on the row', async () => {
    onTestFailed(() => saveFailureShot(page, 'web-e2e-ws-hover'))
    // Expand Ungrouped to reveal the seeded session row, then dwell on it
@@ -376,8 +406,8 @@ describe('web e2e: workspace management (create / rename / flat view / hover car

  it.skipIf(MODE === 'record')('issued zero model calls and stayed clean', async () => {
    expect(tripwire.warnings).toEqual([])
-    // This spec mints no fixture directory contents of its own; the seed it
-    // reuses is owned (and inventory-guarded) by seeded-history.
-    await assertFixtureInventory(SNAPSHOT_DIR, ['.gitkeep'])
+    // The directory-browser aria golden is this spec's one owned artifact;
+    // the seed it reuses is owned (and inventory-guarded) by seeded-history.
+    await assertFixtureInventory(SNAPSHOT_DIR, ['.gitkeep', 'directory-browser.expected.md'])
  })
 })
--- a/packages/host/directory-picker-browse/src/client/DirectoryBrowser.module.css
+++ b/packages/host/directory-picker-browse/src/client/DirectoryBrowser.module.css
@@ -39,6 +39,18 @@
  min-height: 20px;
 }

+/* Deep chains scroll inside the trail (the effect pins the tail into view)
+ * so the edit zone to the right never leaves the bar. */
+.crumbTrail {
+  display: flex;
+  align-items: center;
+  gap: 4px;
+  flex: 0 1 auto;
+  min-width: 0;
+  overflow-x: auto;
+  scrollbar-width: none;
+}
+
 .crumbSeat {
  display: inline-flex;
  align-items: center;
@@ -74,7 +86,7 @@
 /* The empty remainder of the bar: invisible, but a real click target that
 * flips the bar into path-edit mode. */
 .crumbEditZone {
-  flex: 1 1 0;
+  flex: 1 0 34px;
  min-width: 34px;
  align-self: stretch;
  border: none;
--- 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)
+  // 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)

  /** Replace the whole view with one freshly listed level (no selection). */
  const navigate = useCallback((path?: string) => {
@@ -213,16 +216,24 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
  }

  // After the hooks: a closed dialog renders nothing and evaluates no copy.
-  if (!open) return null
-
  const crumbSource = child ?? parent
  const crumbs = crumbSource === null ? [] : displayCrumbs(crumbSource, t('browser.home'))
+  const crumbTail = crumbs.at(-1)?.path
+  useEffect(() => {
+    const trail = crumbTrailRef.current
+    if (trail !== null) trail.scrollLeft = trail.scrollWidth
+  }, [crumbTail])
+
+  if (!open) return null
  const twoPane = selected !== null

  return (
    <Modal
      open={open}
-      onClose={onClose}
+      // 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() }}
      title={t('browser.title')}
      className={clsx(css.dialog)}
      headless
@@ -233,19 +244,21 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
          {pathDraft === null
            ? (
              <>
-                {crumbs.map((crumb, index) => (
-                  <span key={crumb.path} className={css.crumbSeat}>
-                    {index > 0 && <IconChevronRightOutline14 size={12} className={css.crumbChevron} />}
-                    <button
-                      type="button"
-                      className={css.crumb}
-                      disabled={busy}
-                      onClick={() => { navigate(crumb.path) }}
-                    >
-                      {crumb.name}
-                    </button>
-                  </span>
-                ))}
+                <span className={css.crumbTrail} ref={crumbTrailRef}>
+                  {crumbs.map((crumb, index) => (
+                    <span key={crumb.path} className={css.crumbSeat}>
+                      {index > 0 && <IconChevronRightOutline14 size={12} className={css.crumbChevron} />}
+                      <button
+                        type="button"
+                        className={css.crumb}
+                        disabled={busy}
+                        onClick={() => { navigate(crumb.path) }}
+                      >
+                        {crumb.name}
+                      </button>
+                    </span>
+                  ))}
+                </span>
                {/* The empty zone right of the crumbs is the path-edit affordance. */}
                <button
                  type="button"
@@ -308,7 +321,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
        <Button
          variant="outline"
          icon={<IconPlusOutline16 size={14} />}
-          disabled={parent === null || busy || folderDraft !== null}
+          disabled={parent === null || busy || loading || folderDraft !== null}
          onClick={() => {
            setFolderDraft('')
            setCreateError(null)
--- a/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx
+++ b/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx
@@ -1,6 +1,6 @@
 // @vitest-environment jsdom
 import { afterEach, describe, expect, it, vi } from 'vitest'
-import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
+import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
 import type { DirectoryListing } from '@deepseek-ai/dsh-client-runtime/client'
 import { DirectoryBrowseError } from '@deepseek-ai/dsh-client-runtime/client'
 import { DirectoryBrowser } from '../src/client/DirectoryBrowser.tsx'
@@ -207,6 +207,63 @@ describe('DirectoryBrowser', () => {
    expect(screen.queryByRole('button', { name: 'browser.home' })).toBeNull()
  })

+  it('scopes Escape to the topmost dialog: the nested create closes first, the browser only after', async () => {
+    const b = mount()
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
+    expect(screen.getByLabelText('browser.folderName')).toBeTruthy()
+    fireEvent.keyDown(document, { key: 'Escape' })
+    // The nested dialog consumed Escape; the browser stays up.
+    expect(screen.queryByLabelText('browser.folderName')).toBeNull()
+    expect(b.onClose).not.toHaveBeenCalled()
+    fireEvent.keyDown(document, { key: 'Escape' })
+    expect(b.onClose).toHaveBeenCalledOnce()
+  })
+
+  it('keeps both dialogs open when Escape lands during an in-flight creation', async () => {
+    let resolve!: (path: string) => void
+    const createDirectory = vi.fn(() => new Promise<string>((settle) => { resolve = 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: 'pending' } })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.create' }))
+    fireEvent.keyDown(document, { key: 'Escape' })
+    // The in-flight fence holds the nested dialog, and the browser must not
+    // fall out from under it either.
+    expect(screen.getByLabelText('browser.folderName')).toBeTruthy()
+    expect(b.onClose).not.toHaveBeenCalled()
+    await act(async () => { resolve(`${HOME}/pending`) })
+  })
+
+  it('keeps New folder disabled while the post-create relist is still loading', async () => {
+    const pending: (() => void)[] = []
+    const b = mount()
+    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
+    // Every listing after the create hangs until drained: the button must not
+    // offer a second create against a target the pending relist/select
+    // sequence is about to change.
+    const fresh: DirectoryListing = {
+      path: `${HOME}/fresh`, home: HOME,
+      crumbs: [...listingFor(HOME).crumbs, { name: 'fresh', path: `${HOME}/fresh`, hidden: false }],
+      entries: [],
+    }
+    b.listDirectory.mockImplementation((path?: string) =>
+      new Promise<DirectoryListing>((settle) => {
+        pending.push(() => { settle(path === `${HOME}/fresh` ? fresh : listingFor(path)) })
+      }))
+    fireEvent.change(screen.getByLabelText('browser.folderName'), { target: { value: 'fresh' } })
+    fireEvent.click(screen.getByRole('button', { name: 'browser.create' }))
+    await waitFor(() => { expect(screen.queryByLabelText('browser.folderName')).toBeNull() })
+    expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.newFolder' }).disabled).toBe(true)
+    // Drain the relist and the follow-up selection listing; only then does
+    // the affordance return.
+    await act(async () => { for (const settle of pending.splice(0)) settle() })
+    await act(async () => { for (const settle of pending.splice(0)) settle() })
+    expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.newFolder' }).disabled).toBe(false)
+  })
+
  it('creates a folder through the nested dialog and lands with it selected', async () => {
    const b = mount()
    await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })