fix(host): address ds-review-bot v7 on the adaptive picker chooser

- resolve.ts: gate the display branch on linux (the native backend drives exactly darwin/win32/linux) and require a zenity/kdialog binary on PATH, probed once at boot (new probe.ts, injected predicate for tests); type bindHost as the webserver schema's closed union. - index.ts: the disposer now joins the removed entry's fiber teardown so unloading the chooser settles only after the backend quiesced; export BACKEND_PACKAGES as the runtime-string source of truth. - verify-cordis-config: a composition mounting -auto must declare both backends as dependencies (negative-tested), since keyless Linux CI only ever resolves browse and would hide a dropped -native dep. - apps/web scaffold: pin -browse via disable+insert (goldens are interaction-specific); fix the stale workspace-flow comment. - docs/module-graph.md regenerated; README + Agent Note document the ssh -L shape, the PATH-only probe, and the new gate (zh pairs re-paired). - composition spec: assert teardown quiescence without a loader await, cover external entry removal, and await the loader's self-dispose disabled-persist so it cannot race temp-dir teardown.
2026-07-29 21:09:02 +08:00
parent 8cec74748b
commit f0f897ef02
15 changed files with 253 additions and 53 deletions
--- a/packages/host/directory-picker-auto/tests/loader-composition.spec.ts
+++ b/packages/host/directory-picker-auto/tests/loader-composition.spec.ts
@@ -3,10 +3,12 @@
 * vendored Loader mounts the webserver row plus the adaptive chooser, and the
 * assertions observe the durable outcome — which backend entry the chooser
 * mounted into the Loader store, the capability the seam then serves, and
- * that disposing the chooser removes the mounted entry again (HMR safety).
+ * that disposing the chooser removes the mounted entry again (HMR safety),
+ * joining the backend's own teardown before the disposer settles.
 */

-import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
+import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
+import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { pathToFileURL } from 'node:url'
@@ -25,21 +27,27 @@ const NATIVE = '@deepseek-ai/dsh-host-directory-picker-native'
 const BROWSE = '@deepseek-ai/dsh-host-directory-picker-browse'

 let root: string | undefined
+let fakeBin: string | undefined
 let context: Context | undefined

 afterEach(async () => {
  vi.unstubAllEnvs()
  await context?.fiber.dispose()
  context = undefined
-  if (root !== undefined) await rm(root, { recursive: true, force: true })
+  for (const dir of [root, fakeBin]) {
+    // maxRetries absorbs teardown stragglers (e.g. an unawaited fiber's late
+    // file handle) that can otherwise race the recursive scan into ENOTEMPTY.
+    if (dir !== undefined) await rm(dir, { recursive: true, force: true, maxRetries: 3, retryDelay: 50 })
+  }
  root = undefined
+  fakeBin = undefined
 })

 /** Write a dist fixture and a two-row cordis.yml (webserver + chooser), then boot it through the real Loader. */
 async function loadComposition(bindHost: '127.0.0.1' | '0.0.0.0'): Promise<{ ctx: Context; configPath: string }> {
  root = await mkdtemp(join(tmpdir(), 'dsh-directory-picker-auto-'))
  const dist = join(root, 'dist')
-  await mkdir(dist)
+  mkdirSync(dist)
  const distIndex = join(dist, 'index.html')
  await writeFile(distIndex, '<head></head><body>shell</body>')
  const configPath = join(root, 'cordis.yml')
@@ -83,17 +91,26 @@ function entryNames(ctx: Context): string[] {
  return [...ctx.loader.entries()].map(entry => entry.options.name)
 }

-/** Force every signal of an attended host: no SSH launch, a display on any platform. */
+/**
+ * Force every signal of an attended host on any platform: no SSH launch, a
+ * display, and a PATH holding one executable chooser binary so the real
+ * probe resolves identically on hosts with and without zenity/kdialog.
+ */
 function stubAttendedHost(): void {
+  fakeBin = mkdtempSync(join(tmpdir(), 'dsh-picker-bin-'))
+  const zenity = join(fakeBin, 'zenity')
+  writeFileSync(zenity, '#!/bin/sh\n')
+  chmodSync(zenity, 0o755)
+  vi.stubEnv('PATH', fakeBin)
  vi.stubEnv('SSH_CONNECTION', '')
  vi.stubEnv('SSH_TTY', '')
  vi.stubEnv('DISPLAY', ':0')
 }

 describe('real Loader composition', () => {
-  // Real-Loader composition resolves workspace packages through tsx at test
-  // time; first resolution after the host/client program split is slow enough
-  // to trip the default 5s budget on cold caches.
+  // The 60s budget covers this file's static imports (webserver plus both
+  // backend node halves through tsx), which dominate on cold caches; the
+  // Loader itself resolves nothing here — `loader.internal` is a module map.
  it('mounts the native backend for an attended loopback host and unmounts it on disposal', { timeout: 60_000 }, async () => {
    stubAttendedHost()
    const { ctx, configPath } = await loadComposition('127.0.0.1')
@@ -110,12 +127,19 @@ describe('real Loader composition', () => {
    // booted config file must never gain the resolved backend row.
    expect(await readFile(configPath, 'utf8')).not.toContain(NATIVE)

-    // HMR safety: disposing the chooser's fiber removes the entry it created.
+    // HMR safety: disposing the chooser's fiber removes the entry it created,
+    // and the disposer joins the backend's teardown — the service is gone the
+    // moment dispose() settles, with no further loader await.
    const autoEntry = [...ctx.loader.entries()].find(entry => entry.options.name === AUTO)!
    await autoEntry.fiber!.dispose()
-    await ctx.loader.await()
    expect(entryNames(ctx)).not.toContain(NATIVE)
    expect(ctx.get('directoryPicker')).toBeUndefined()
+    // Self-disposing an include-tree entry persists `disabled: true` (loader
+    // behavior, not the chooser's); await that debounced write so it cannot
+    // race the temp-dir removal, and pin that the persisted row is the
+    // chooser itself — the resolved backend still never reaches the file.
+    await expect.poll(async () => await readFile(configPath, 'utf8')).toContain('disabled: true')
+    expect(await readFile(configPath, 'utf8')).not.toContain(NATIVE)
  })

  it('mounts the browse backend under an SSH launch', { timeout: 60_000 }, async () => {
@@ -136,4 +160,17 @@ describe('real Loader composition', () => {
    expect(entryNames(ctx)).toContain(BROWSE)
    expect(entryNames(ctx)).not.toContain(NATIVE)
  })
+
+  it('tolerates the mounted entry being removed by the tree before the chooser unloads', { timeout: 60_000 }, async () => {
+    stubAttendedHost()
+    const { ctx, configPath } = await loadComposition('127.0.0.1')
+
+    const backendEntry = [...ctx.loader.entries()].find(entry => entry.options.name === NATIVE)!
+    ctx.loader.remove(backendEntry.id)
+    const autoEntry = [...ctx.loader.entries()].find(entry => entry.options.name === AUTO)!
+    await expect(autoEntry.fiber!.dispose()).resolves.not.toThrow()
+    expect(entryNames(ctx)).not.toContain(NATIVE)
+    // Same self-dispose persistence as above: let the write land before teardown.
+    await expect.poll(async () => await readFile(configPath, 'utf8')).toContain('disabled: true')
+  })
 })
--- a/packages/host/directory-picker-auto/tests/resolve.spec.ts
+++ b/packages/host/directory-picker-auto/tests/resolve.spec.ts
@@ -1,12 +1,17 @@
-import { describe, expect, it } from 'vitest'
+import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { delimiter, join } from 'node:path'
+import { afterEach, describe, expect, it } from 'vitest'
+import { canExecute, hasLinuxChooserBinary } from '../src/probe.ts'
 import { resolveDirectoryPickerBackend } from '../src/resolve.ts'
 import type { DirectoryPickerHostFacts } from '../src/resolve.ts'

-/** Baseline facts that resolve to `native`; each case overrides one signal. */
+/** Baseline facts that resolve to `native`; each case overrides one signal (darwin never consults `linuxChooser`). */
 const attended: DirectoryPickerHostFacts = {
  bindHost: '127.0.0.1',
  platform: 'darwin',
  env: {},
+  linuxChooser: false,
 }

 describe('resolveDirectoryPickerBackend', () => {
@@ -24,14 +29,63 @@ describe('resolveDirectoryPickerBackend', () => {
    expect(resolveDirectoryPickerBackend({ ...attended, env: { SSH_TTY: '/dev/pts/3' } })).toBe('browse')
  })

-  it('requires a display session on platforms without an implied one', () => {
-    expect(resolveDirectoryPickerBackend({ ...attended, platform: 'linux' })).toBe('browse')
-    expect(resolveDirectoryPickerBackend({ ...attended, platform: 'linux', env: { DISPLAY: ':0' } })).toBe('native')
-    expect(resolveDirectoryPickerBackend({ ...attended, platform: 'linux', env: { WAYLAND_DISPLAY: 'wayland-1' } })).toBe('native')
+  it('requires a display session and a chooser binary on linux', () => {
+    const linux: DirectoryPickerHostFacts = { ...attended, platform: 'linux', linuxChooser: true }
+    expect(resolveDirectoryPickerBackend(linux)).toBe('browse')
+    expect(resolveDirectoryPickerBackend({ ...linux, env: { DISPLAY: ':0' } })).toBe('native')
+    expect(resolveDirectoryPickerBackend({ ...linux, env: { WAYLAND_DISPLAY: 'wayland-1' } })).toBe('native')
+    expect(resolveDirectoryPickerBackend({ ...linux, env: { DISPLAY: ':0' }, linuxChooser: false })).toBe('browse')
+  })
+
+  it('resolves browse on platforms the native backend cannot serve, display or not', () => {
+    expect(resolveDirectoryPickerBackend({ ...attended, platform: 'freebsd', env: { DISPLAY: ':0' }, linuxChooser: true })).toBe('browse')
+    expect(resolveDirectoryPickerBackend({ ...attended, platform: 'openbsd', env: { WAYLAND_DISPLAY: 'wayland-1' } })).toBe('browse')
  })

  it('treats blank env exports as unset', () => {
    expect(resolveDirectoryPickerBackend({ ...attended, env: { SSH_CONNECTION: '', SSH_TTY: '' } })).toBe('native')
-    expect(resolveDirectoryPickerBackend({ ...attended, platform: 'linux', env: { DISPLAY: '', WAYLAND_DISPLAY: '' } })).toBe('browse')
+    expect(resolveDirectoryPickerBackend({
+      ...attended, platform: 'linux', linuxChooser: true, env: { DISPLAY: '', WAYLAND_DISPLAY: '' },
+    })).toBe('browse')
+  })
+})
+
+let probeRoot: string | undefined
+
+afterEach(() => {
+  if (probeRoot !== undefined) rmSync(probeRoot, { recursive: true, force: true })
+  probeRoot = undefined
+})
+
+describe('hasLinuxChooserBinary', () => {
+  it('finds a chooser binary in any PATH segment, skipping empty segments', () => {
+    const seen: string[] = []
+    const path = ['', '/opt/none', '/usr/local/bin'].join(delimiter)
+    const found = hasLinuxChooserBinary(path, (candidate) => {
+      seen.push(candidate)
+      return candidate === join('/usr/local/bin', 'kdialog')
+    })
+    expect(found).toBe(true)
+    expect(seen).toEqual([
+      join('/opt/none', 'zenity'), join('/opt/none', 'kdialog'),
+      join('/usr/local/bin', 'zenity'), join('/usr/local/bin', 'kdialog'),
+    ])
+  })
+
+  it('reports absence when no segment holds a chooser binary', () => {
+    expect(hasLinuxChooserBinary(['/a', '/b'].join(delimiter), () => false)).toBe(false)
+    expect(hasLinuxChooserBinary('', () => true)).toBe(false)
+    expect(hasLinuxChooserBinary(undefined, () => true)).toBe(false)
+  })
+})
+
+describe('canExecute', () => {
+  it('accepts an executable file and rejects an absent one', () => {
+    probeRoot = mkdtempSync(join(tmpdir(), 'dsh-picker-probe-'))
+    const binary = join(probeRoot, 'zenity')
+    writeFileSync(binary, '#!/bin/sh\n')
+    chmodSync(binary, 0o755)
+    expect(canExecute(binary)).toBe(true)
+    expect(canExecute(join(probeRoot, 'kdialog'))).toBe(false)
  })
 })