fix(picker): correct crash-isolation and DPI claims, wire the built-worker guard, and tidy round-four nits
The round-four review's v6 pass found three factual gaps and the v5 pass two nits. Correct them before merge: - The in-process note claimed a koffi signature mistake is 'contained to the worker thread' — worker_threads share the process, so a native access violation takes down the whole Node process with no PowerShell fallback. State the real blast radius and record the deferred pkg-VFS worker-spawn arm in Consequences (both languages, pairing re-recorded). - The 2026-07-27 picker note claimed unconditional 'per-monitor-v2 DPI'; PMv2-less hosts (Server 2016 / Win10 1607) cascade to per-monitor or system-aware. Say 'the best thread DPI awareness the host accepts' (both languages, pairing re-recorded). - built-worker.e2e.ts was not in any keyless gate (vitest.e2e config is not part of the default unit run and builtBinSmokeGate's explicit list missed it), so lib/worker.cjs load regressions passed keyless CI. Add it to builtBinSmokeGate alongside the workflow-workerthread sibling. - Remove the dead trailing 'void bindings' in win32-dialog-logic.spec.ts and give native-picker.spec.ts the sibling module header it lacked.
This commit is contained in:
Huanqi Cao
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-27-native-workspace-directory-picker.md
|
||||
2026-07-27-native-workspace-directory-picker.md: 45fa77b5519179e006f9109846a1602e6e22a6e2
|
||||
2026-07-27-native-workspace-directory-picker.zh.md: 2d6800d20b1f0dfe0b20ac9a5c90037599ece32a
|
||||
2026-07-27-native-workspace-directory-picker.md: 452ec60371558de79dbff964a12d96d2150dc6f6
|
||||
2026-07-27-native-workspace-directory-picker.zh.md: c3e6b8825e78201ce791cff51869aade67d30a5e
|
||||
|
||||
@@ -30,7 +30,7 @@ The native dialog RPC is accepted only from a loopback socket with same-origin b
|
||||
Platform adapters open the dialog without a shell — spawned native tools on POSIX, an in-process COM conversation on Windows:
|
||||
|
||||
- macOS: `osascript` and the system folder chooser.
|
||||
- Windows: the in-process koffi `IFileOpenDialog` worker with per-monitor-v2 DPI ([in-process dialog note](2026-08-02-win32-in-process-folder-dialog.md)); the PowerShell chain (`pwsh` in STA mode, then Windows PowerShell 5.1, both DPI-corrected) remains the fallback ([picker fix](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md)).
|
||||
- Windows: the in-process koffi `IFileOpenDialog` worker with the best thread DPI awareness the host accepts (per-monitor-v2 when available; PMv2-less hosts cascade to per-monitor or system-aware) ([in-process dialog note](2026-08-02-win32-in-process-folder-dialog.md)); the PowerShell chain (`pwsh` in STA mode, then Windows PowerShell 5.1, both DPI-corrected) remains the fallback ([picker fix](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md)).
|
||||
- Linux: `zenity`, with `kdialog` as a fallback when Zenity is unavailable.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
@@ -30,7 +30,7 @@ Status: implemented
|
||||
平台适配器不经 shell 打开对话框——POSIX 上 spawn 原生工具，Windows 上是进程内 COM 会话：
|
||||
|
||||
- macOS：`osascript` 和系统文件夹选择器。
|
||||
- Windows：进程内 koffi `IFileOpenDialog` worker，带 per-monitor-v2 DPI（见[进程内对话框 Note](2026-08-02-win32-in-process-folder-dialog.md)）；PowerShell 链（STA 模式的 `pwsh`，再到 Windows PowerShell 5.1，均已修正 DPI）保留为回退（见[选择器修复](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md)）。
|
||||
- Windows：进程内 koffi `IFileOpenDialog` worker，使用宿主接受的最佳线程 DPI 感知（可用时为 per-monitor-v2；不支持 PMv2 的主机级联到 per-monitor 或 system-aware）（见[进程内对话框 Note](2026-08-02-win32-in-process-folder-dialog.md)）；PowerShell 链（STA 模式的 `pwsh`，再到 Windows PowerShell 5.1，均已修正 DPI）保留为回退（见[选择器修复](../bug-fix/2026-08-01-windows-picker-pwsh-dpi.md)）。
|
||||
- Linux：使用 `zenity`；Zenity 不可用时回退到 `kdialog`。
|
||||
|
||||
## 考虑过的替代方案
|
||||
|
||||
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-08-02-win32-in-process-folder-dialog.md
|
||||
2026-08-02-win32-in-process-folder-dialog.md: 96bc213ea7cddef6223aa3be69e56688dc9c4724
|
||||
2026-08-02-win32-in-process-folder-dialog.zh.md: a3dfad2ef73cb28ae8c4c47b6380740aab6de5c3
|
||||
2026-08-02-win32-in-process-folder-dialog.md: e18ecd1d2e79ec39a265d3d93913beaa7530e645
|
||||
2026-08-02-win32-in-process-folder-dialog.zh.md: f146d61f6378062886db732e6884594d56b3d170
|
||||
|
||||
@@ -23,4 +23,5 @@ The Windows directory picker's primary tier was a spawned PowerShell script arou
|
||||
|
||||
- Every Windows machine gets the modern dialog with the best DPI awareness it supports (per-monitor-v2 on 1703+), PowerShell installed or not; the PowerShell tiers only serve hosts where koffi cannot drive COM.
|
||||
- Real dialog rendering and the selection path stay a manual Windows check (the auto-close smoke proves open/abort/unwind); a wedged abort can leak one dialog thread until process exit, documented in the package README.
|
||||
- The COM vtable slots and GUIDs used are frozen Windows ABI (Vista); a koffi signature mistake is an in-process crash risk contained to the worker thread and caught by the win32 smoke before shipping.
|
||||
- The COM vtable slots and GUIDs used are frozen Windows ABI (Vista); a koffi signature mistake is a native-crash risk that can take down the whole Node process — `worker_threads` share the process, so an access violation is not contained to the worker and no PowerShell fallback runs. The mocked-koffi ABI pins and the real win32 smoke exist to catch such mistakes before shipping.
|
||||
- The packaged-binary VFS arm — resolution of `./worker.cjs` inside a pkg snapshot — is not exercised by any automated test: the source worker and the built `lib/worker.cjs` under plain Node are covered, and the VFS-specific spawn remains deferred to the Windows CI roadmap.
|
||||
|
||||
@@ -23,4 +23,5 @@ Windows 目录选择器的主层此前是围绕 WinForms `FolderBrowserDialog`
|
||||
|
||||
- 每台 Windows 机器都得到带其所支持的最佳 DPI 感知（1703+ 为 per-monitor-v2）的现代对话框，无论是否安装 PowerShell；PowerShell 层只服务 koffi 无法驱动 COM 的主机。
|
||||
- 真实对话框渲染与选中路径仍是手动 Windows 检查（自动关闭冒烟证明打开／中止／收尾）；卡死的中止可能泄漏一个对话框线程直到进程退出，已记录于包 README。
|
||||
- 所用 COM vtable 槽位与 GUID 是冻结的 Windows ABI（Vista 起）；koffi 签名错误是被限制在 worker 线程内的进程内崩溃风险，并在交付前被 win32 冒烟捕获。
|
||||
- 所用 COM vtable 槽位与 GUID 是冻结的 Windows ABI（Vista 起）；koffi 签名错误是可能拖垮整个 Node 进程的原生崩溃风险——`worker_threads` 与主线程共享进程，访问冲突不会只局限在 worker 内，也不会进入 PowerShell 回退。mocked-koffi 的 ABI 钉与真实 win32 冒烟正是为了在交付前捕获这类错误。
|
||||
- 打包二进制的 VFS 臂——在 pkg 快照内解析 `./worker.cjs`——不受任何自动化测试覆盖：源码 worker 与普通 Node 下构建出的 `lib/worker.cjs` 已被覆盖，VFS 专属的 spawn 推迟到 Windows CI 路线图。
|
||||
|
||||
@@ -1,3 +1,10 @@
|
||||
/**
|
||||
* Native picker tier selection and the execFile adapter: the in-process
|
||||
* dialog primary, the pwsh → Windows PowerShell 5.1 fallback chain (any
|
||||
* non-abort pwsh failure cascades), the abort-never-falls-through rule, and
|
||||
* the triple-miss AggregateError carrying the dialog/pwsh/5.1 causes.
|
||||
*/
|
||||
|
||||
type ExecFileCallback = (
|
||||
error: (Error & { code?: string | number }) | null,
|
||||
stdout: string,
|
||||
|
||||
@@ -94,6 +94,5 @@ describe('runFolderDialog', () => {
|
||||
expect(() => runFolderDialog(bindings, 'Pick', vi.fn())).toThrow(`${what} failed: HRESULT 0x80004005`)
|
||||
expect(dialog.release).toHaveBeenCalledOnce()
|
||||
expect(uninitialize).toHaveBeenCalledOnce()
|
||||
void bindings
|
||||
})
|
||||
})
|
||||
|
||||
@@ -597,6 +597,7 @@ function builtBinSmokeGate(needs: string[] = ['build']): Gate {
|
||||
'apps/cli/tests/built-bin.e2e.ts',
|
||||
'packages/examples/cli-demo/tests/built-bin.e2e.ts',
|
||||
'packages/examples/acp-demo/tests/built-bin.e2e.ts',
|
||||
'packages/host/directory-picker-native/tests/built-worker.e2e.ts',
|
||||
'packages/ui/jsonrpc/tests/built-scope-carrier.e2e.ts',
|
||||
// The worker-entry packages' built bundles: the only automated proof
|
||||
// that lib/index.js resolves its sibling lib/worker.cjs under plain node
|
||||
|
||||
Reference in New Issue
Block a user